Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rate limit support #67

Merged
merged 7 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions lib/inngest/error.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ defmodule Inngest.RetryAfterError do
defexception [:message, seconds: 3]
end

defmodule Inngest.InvalidDebounceConfigError do
defexception message: "a 'period' must be set for debounce"
defmodule Inngest.DebounceConfigError do
defexception [:message]
end

defmodule Inngest.BatchEventConfigError do
defexception [:message]
end

defmodule Inngest.InvalidBatchEventConfigError do
defmodule Inngest.RateLimitConfigError do
defexception [:message]
end
6 changes: 6 additions & 0 deletions lib/inngest/function.ex
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ defmodule Inngest.Function do
}
|> maybe_debounce()
|> maybe_batch_events()
|> maybe_rate_limit()
] ++ handler
end

Expand All @@ -182,6 +183,11 @@ defmodule Inngest.Function do
|> Inngest.FnOpts.validate_batch_events(config)
end

defp maybe_rate_limit(config) do
fn_opts()
|> Inngest.FnOpts.validate_rate_limit(config)
end

defp fn_opts() do
case __MODULE__.__info__(:attributes) |> Keyword.get(:func) |> List.first() do
nil -> %Inngest.FnOpts{}
Expand Down
73 changes: 56 additions & 17 deletions lib/inngest/function/config.ex
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
defmodule Inngest.FnOpts do
@moduledoc false
@moduledoc """
Function configuration options
"""

defstruct [
:id,
:name,
:debounce,
:batch_events,
:rate_limit,
retries: 3
]

Expand All @@ -16,11 +19,12 @@
name: binary(),
retries: number() | nil,
debounce: debounce() | nil,
batch_events: batch_events() | nil
batch_events: batch_events() | nil,
rate_limit: rate_limit() | nil
}

@type debounce() :: %{
key: nil | binary(),
key: binary() | nil,
period: binary()
}

Expand All @@ -29,6 +33,12 @@
timeout: binary()
}

@type rate_limit() :: %{
limit: number(),
period: binary(),
key: binary() | nil
}

@doc """
Validate the debounce configuration
"""
Expand All @@ -42,17 +52,17 @@
period = Map.get(debounce, :period)

if is_nil(period) do
raise Inngest.InvalidDebounceConfigError
raise Inngest.DebounceConfigError, message: "'period' must be set for debounce"
end

case Util.parse_duration(period) do
{:error, error} ->
raise Inngest.InvalidDebounceConfigError, message: error
raise Inngest.DebounceConfigError, message: error

{:ok, seconds} ->
# credo:disable-for-next-line
if seconds > 7 * Util.day_in_seconds() do
raise Inngest.InvalidDebounceConfigError,
raise Inngest.DebounceConfigError,
message: "cannot specify period for more than 7 days"
end
end
Expand All @@ -74,30 +84,59 @@
max_size = Map.get(batch, :max_size)
timeout = Map.get(batch, :timeout)

if is_nil(max_size) do
raise Inngest.InvalidBatchEventConfigError,
message: "'max_size' must be set for batch_events"
end

if is_nil(timeout) do
raise Inngest.InvalidBatchEventConfigError,
message: "'timeout' must be set for batch_events"
if is_nil(max_size) || is_nil(timeout) do
raise Inngest.BatchEventConfigError,
message: "'max_size' and 'timeout' must be set for batch_events"
end

case Util.parse_duration(timeout) do
{:error, error} ->
raise Inngest.InvalidBatchEventConfigError, message: error
raise Inngest.BatchEventConfigError, message: error

{:ok, seconds} ->
# credo:disable-for-next-line
if seconds < 1 || seconds > 60 do
raise Inngest.InvalidBatchEventConfigError,
raise Inngest.BatchEventConfigError,
message: "'timeout' duration set to '#{timeout}', needs to be 1s - 60s"
end
end

batch = batch |> Map.put(:maxSize, max_size) |> Map.drop([:max_size])
batch = %{maxSize: max_size, timeout: timeout}
Map.put(config, :batchEvents, batch)
end
end

@doc """
Validate the rate limit config
"""
@spec validate_rate_limit(t(), map()) :: map()
def validate_rate_limit(fnopts, config) do
case fnopts |> Map.get(:rate_limit) do
nil ->
config

rate_limit ->
limit = Map.get(rate_limit, :limit)
period = Map.get(rate_limit, :period)

if is_nil(limit) || is_nil(period) do
raise Inngest.RateLimitConfigError,
message: "'limit' and 'period' must be set for rate_limit"
end

case Util.parse_duration(period) do
{:error, error} ->
raise Inngest.RateLimitConfigError, message: error

Check warning on line 129 in lib/inngest/function/config.ex

View check run for this annotation

Codecov / codecov/patch

lib/inngest/function/config.ex#L129

Added line #L129 was not covered by tests

{:ok, seconds} ->
# credo:disable-for-next-line
if seconds < 1 || seconds > 60 do
raise Inngest.RateLimitConfigError,
message: "'period' duration set to '#{period}', needs to be 1s - 60s"
end
end

Map.put(config, :rateLimit, rate_limit)
end
end
end
2 changes: 2 additions & 0 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ defmodule Inngest.MixProject do
Function: [
Inngest.Event,
Inngest.Function,
Inngest.FnOpts,
Inngest.Trigger,
Inngest.Function.Step,
Inngest.Function.Context,
Inngest.Function.Input
],
Router: [
Expand Down
5 changes: 2 additions & 3 deletions test/inngest/function/cases/batch_events_test.exs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
defmodule Inngest.Function.Cases.BatchEventsTest do
use ExUnit.Case, async: true

alias Inngest.Test.DevServer
import Inngest.Test.Helper

# TODO: Add test after moving batching logic to OSS
# alias Inngest.Test.DevServer
# import Inngest.Test.Helper
end
36 changes: 36 additions & 0 deletions test/inngest/function/cases/rate_limit_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
defmodule Inngest.Function.Cases.RateLimitTest do
use ExUnit.Case, async: true

alias Inngest.Test.DevServer
import Inngest.Test.Helper

@default_sleep 10_000

@tag :integration
test "should only run 2 out of 10" do
event_ids = Enum.map(1..10, fn _ -> send_test_event("test/plug.ratelimit") end)

Process.sleep(@default_sleep)

fn_runs =
event_ids
|> Enum.map(fn id ->
{:ok, %{"data" => data}} = DevServer.run_ids(id)

if Enum.count(data) == 1 do
assert [
%{
"output" => "Rate Limited",
"status" => "Completed",
"run_id" => run_id

Check warning on line 25 in test/inngest/function/cases/rate_limit_test.exs

View workflow job for this annotation

GitHub Actions / Test (Elixir 1.14 / OTP 24.3)

variable "run_id" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 25 in test/inngest/function/cases/rate_limit_test.exs

View workflow job for this annotation

GitHub Actions / Test (Elixir 1.14 / OTP 25.3)

variable "run_id" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 25 in test/inngest/function/cases/rate_limit_test.exs

View workflow job for this annotation

GitHub Actions / Test (Elixir 1.15 / OTP 24.3)

variable "run_id" is unused (if the variable is not meant to be used, prefix it with an underscore)

Check warning on line 25 in test/inngest/function/cases/rate_limit_test.exs

View workflow job for this annotation

GitHub Actions / Test (Elixir 1.15 / OTP 25.3)

variable "run_id" is unused (if the variable is not meant to be used, prefix it with an underscore)
}
] = data
else
nil
end
end)
|> Enum.filter(&(!is_nil(&1)))

assert Enum.count(fn_runs) <= 2
end
end
76 changes: 64 additions & 12 deletions test/inngest/function/config_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,29 @@ defmodule Inngest.FnOptsTest do
assert %{debounce: %{period: "5s"}} = FnOpts.validate_debounce(@fn_opts, @config)
end

## Invalid configs
## configs
test "should raise when period is missing" do
opts = drop_at(@fn_opts, [:debounce, :period])

assert_raise Inngest.InvalidDebounceConfigError, fn ->
FnOpts.validate_debounce(opts, @config)
end
assert_raise Inngest.DebounceConfigError,
"'period' must be set for debounce",
fn ->
FnOpts.validate_debounce(opts, @config)
end
end

test "should raise with invalid period" do
opts = update_at(@fn_opts, [:debounce, :period], "yolo")

assert_raise Inngest.InvalidDebounceConfigError, fn ->
assert_raise Inngest.DebounceConfigError, "invalid duration: 'yolo'", fn ->
FnOpts.validate_debounce(opts, @config)
end
end

test "should raise with period longer than 7 days" do
opts = update_at(@fn_opts, [:debounce, :period], "8d")

assert_raise Inngest.InvalidDebounceConfigError, fn ->
assert_raise Inngest.DebounceConfigError, fn ->
FnOpts.validate_debounce(opts, @config)
end
end
Expand Down Expand Up @@ -77,8 +79,8 @@ defmodule Inngest.FnOptsTest do
test "should raise if max_size is missing" do
opts = drop_at(@fn_opts, [:batch_events, :max_size])

assert_raise Inngest.InvalidBatchEventConfigError,
"'max_size' must be set for batch_events",
assert_raise Inngest.BatchEventConfigError,
"'max_size' and 'timeout' must be set for batch_events",
fn ->
FnOpts.validate_batch_events(opts, @config)
end
Expand All @@ -87,8 +89,8 @@ defmodule Inngest.FnOptsTest do
test "should raise if timeout is missing" do
opts = drop_at(@fn_opts, [:batch_events, :timeout])

assert_raise Inngest.InvalidBatchEventConfigError,
"'timeout' must be set for batch_events",
assert_raise Inngest.BatchEventConfigError,
"'max_size' and 'timeout' must be set for batch_events",
fn ->
FnOpts.validate_batch_events(opts, @config)
end
Expand All @@ -97,7 +99,7 @@ defmodule Inngest.FnOptsTest do
test "should raise if timeout is invalid" do
opts = update_at(@fn_opts, [:batch_events, :timeout], "hello")

assert_raise Inngest.InvalidBatchEventConfigError,
assert_raise Inngest.BatchEventConfigError,
"invalid duration: 'hello'",
fn ->
FnOpts.validate_batch_events(opts, @config)
Expand All @@ -107,11 +109,61 @@ defmodule Inngest.FnOptsTest do
test "should raise if timeout is out of range" do
opts = update_at(@fn_opts, [:batch_events, :timeout], "2m")

assert_raise Inngest.InvalidBatchEventConfigError,
assert_raise Inngest.BatchEventConfigError,
"'timeout' duration set to '2m', needs to be 1s - 60s",
fn ->
FnOpts.validate_batch_events(opts, @config)
end
end
end

describe "validate_rate_limit/2" do
@fn_opts %FnOpts{
id: "foobar",
name: "Foobar",
rate_limit: %{
limit: 10,
period: "5s"
}
}

test "should succeed with valid config" do
assert %{
rateLimit: %{
limit: 10,
period: "5s"
}
} = FnOpts.validate_rate_limit(@fn_opts, @config)
end

test "should raise when limit is missing" do
opts = drop_at(@fn_opts, [:rate_limit, :limit])

assert_raise Inngest.RateLimitConfigError,
"'limit' and 'period' must be set for rate_limit",
fn ->
FnOpts.validate_rate_limit(opts, @config)
end
end

test "should raise when period is missing" do
opts = drop_at(@fn_opts, [:rate_limit, :period])

assert_raise Inngest.RateLimitConfigError,
"'limit' and 'period' must be set for rate_limit",
fn ->
FnOpts.validate_rate_limit(opts, @config)
end
end

test "should raise if timeout is out of range" do
opts = update_at(@fn_opts, [:rate_limit, :period], "2m")

assert_raise Inngest.RateLimitConfigError,
"'period' duration set to '2m', needs to be 1s - 60s",
fn ->
FnOpts.validate_rate_limit(opts, @config)
end
end
end
end
21 changes: 21 additions & 0 deletions test/support/cases/rate_limit_fn.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
defmodule Inngest.Test.Case.RateLimitFn do
@moduledoc false

use Inngest.Function
alias Inngest.{FnOpts, Trigger}

@func %FnOpts{
id: "ratelimit-fn",
name: "RateLimit Function",
rate_limit: %{
limit: 2,
period: "5s"
}
}
@trigger %Trigger{event: "test/plug.ratelimit"}

@impl true
def exec(_ctx, _args) do
{:ok, "Rate Limited"}
end
end
Loading