Compare commits

..

No commits in common. "mistress" and "api-surface" have entirely different histories.

5 changed files with 14 additions and 142 deletions

View file

@ -11,10 +11,7 @@ defmodule Dexcord.Api do
flows through the limiter: acquire a token (sleeping in the caller's process
while `{:wait, ms}`), issue the request, feed the response headers back for
bucket learning, and on a 429 honor `retry_after` / the global scope and retry
up to #{3} times. Connection-level transport faults (`%Mint.TransportError{}`,
e.g. a stale pooled keep-alive the server already closed) share that retry
budget with a short backoff; timeouts and pool errors are NOT retried, since
a timed-out request may have been processed and retrying could double-post.
up to #{3} times.
All bodies are string-keyed maps; JSON encode/decode uses the built-in `JSON`
module. Returns `{:ok, map}`, `{:ok, nil}` (204 / empty 2xx body),
@ -48,16 +45,6 @@ defmodule Dexcord.Api do
@default_base_url "https://discord.com/api/v10"
@user_agent "DiscordBot (https://github.com/luna/dexcord, 0.1.0)"
@max_retries 3
# Backoff (ms) before transport-error retry N. The first retry is immediate:
# the dominant fault is a stale pooled keep-alive, and the retry opens a
# fresh connection anyway.
@transport_retry_ms {0, 250, 1000}
# Connection-fault reasons safe to retry: the request (almost certainly)
# never reached the server. Deliberately excludes :timeout — a timed-out
# request may have been fully processed, and retrying it can double-post.
# Finch wraps Mint's error as `%Finch.TransportError{reason: ..., source: ...}`
# with the same reason atoms.
@retryable_transport_reasons [:closed, :econnrefused, :econnreset, :epipe]
# Small cushion added to every computed sleep so we wake just after a window
# or retry-after boundary rather than a hair before it.
@wait_padding_ms 20
@ -85,9 +72,6 @@ defmodule Dexcord.Api do
rate-limit deadline (`status: nil`, message `"rate limit deadline exceeded"`)
when the internal waits would exceed `:api_deadline_ms`. For a non-JSON
error body a bounded snippet of the raw body is kept in `:message`.
Connection-level `%Mint.TransportError{}` faults are retried up to
#{3} times before surfacing here (`status: nil`, message is the
inspected reason).
"""
@spec request(
atom(),
@ -134,9 +118,6 @@ defmodule Dexcord.Api do
{:retry_429, headers, resp_body} ->
handle_429(method, path, body, opts, route, attempt, headers, resp_body, deadline)
{:retry_transport, reason} ->
handle_transport_error(method, path, body, opts, route, attempt, reason, deadline)
{:done, done} ->
done
end
@ -160,41 +141,11 @@ defmodule Dexcord.Api do
Ratelimit.update(route, resp_headers)
{:done, {:error, error(resp)}}
{:error, %struct{reason: r} = reason}
when struct in [Finch.TransportError, Mint.TransportError] and
r in @retryable_transport_reasons ->
{:retry_transport, reason}
{:error, reason} ->
{:done, {:error, transport_error(reason)}}
{:done, {:error, %Error{status: nil, code: nil, message: inspect(reason), errors: nil}}}
end
end
# Connection-level faults (a stale pooled keep-alive the server already
# closed, a refused/reset connection) almost always mean the request never
# reached Discord, so a bounded retry is safe. Timeouts and pool errors are
# NOT retried: a timed-out POST may have been fully processed, and retrying
# it can double-post.
defp handle_transport_error(method, path, body, opts, route, attempt, _reason, deadline)
when attempt < @max_retries do
backoff = elem(@transport_retry_ms, min(attempt, tuple_size(@transport_retry_ms) - 1))
if mono_now() + backoff >= deadline do
deadline_error()
else
if backoff > 0, do: Process.sleep(backoff)
do_request(method, path, body, opts, route, attempt + 1, deadline)
end
end
defp handle_transport_error(_method, _path, _body, _opts, _route, _attempt, reason, _deadline) do
{:error, transport_error(reason)}
end
defp transport_error(reason) do
%Error{status: nil, code: nil, message: inspect(reason), errors: nil}
end
defp handle_429(method, path, body, opts, route, attempt, headers, resp_body, deadline)
when attempt < @max_retries do
retry_ms = retry_after_ms(resp_body, headers)
@ -531,13 +482,10 @@ defmodule Dexcord.Api do
after_id,
@history_page_size,
fn cursor ->
case Dexcord.Api.Messages.get_channel_messages(channel_id,
limit: @history_page_size,
after: cursor
) do
{:ok, messages} -> {:ok, Enum.reverse(messages)}
{:error, _} = error -> error
end
Dexcord.Api.Messages.get_channel_messages(channel_id,
limit: @history_page_size,
after: cursor
)
end,
fn last -> last.id end
)

View file

@ -53,9 +53,7 @@ defmodule Dexcord.Supervisor do
[
Dexcord.Session,
cache_and_dispatcher(),
# conn_max_idle_time keeps pooled keep-alives younger than Cloudflare's
# idle reap, so requests rarely check out an already-closed connection.
{Finch, name: Dexcord.Finch, pools: %{default: [conn_max_idle_time: 45_000]}},
{Finch, name: Dexcord.Finch},
Dexcord.Api.Ratelimit,
{Task.Supervisor, name: Dexcord.TaskSupervisor}
] ++

View file

@ -143,37 +143,6 @@ defmodule Dexcord.ApiIntegrationTest do
assert elapsed_us >= 200_000
end
test "a closed connection is retried and then succeeds" do
FakeRest.stub(:post, "/channels/5/messages", FakeRest.close())
FakeRest.stub(
:post,
"/channels/5/messages",
ok_json(~s({"id":"ok"}), bucket: "T", remaining: 5, reset_after: 1.0)
)
assert {:ok, %Dexcord.Message{id: "ok"}} = Api.create_message(5, "hi")
# Exactly two hits: the aborted attempt and the successful retry.
assert_receive {:rest_hit, _}
assert_receive {:rest_hit, _}
refute_receive {:rest_hit, _}, 50
end
test "transport errors give up after max retries and surface the reason" do
# Sticky stub: every attempt gets its connection slammed shut.
FakeRest.stub(:post, "/channels/5/messages", FakeRest.close())
assert {:error, %Error{status: nil, code: nil, message: message}} =
Api.create_message(5, "hi")
assert message =~ ":closed"
# Initial attempt + 3 retries (backoffs up to 1s), then no more.
for _ <- 1..4, do: assert_receive({:rest_hit, _}, 2_000)
refute_receive {:rest_hit, _}, 50
end
test "a global 429 blocks an unrelated route until the lock expires" do
global_429 = ~s({"message":"global rate limit","retry_after":0.4,"global":true})

View file

@ -89,8 +89,8 @@ defmodule Dexcord.PaginationTest do
end
test "after: flips to ascending paging with after= cursor" do
page1_ids = Enum.to_list(200..101//-1)
page2_ids = [203, 202, 201]
page1_ids = Enum.to_list(101..200)
page2_ids = [201, 202, 203]
assert length(page1_ids) == 100
FakeRest.stub(
@ -107,7 +107,7 @@ defmodule Dexcord.PaginationTest do
result = Api.message_history(2, after: 100) |> Enum.to_list()
assert Enum.map(result, & &1.id) == Enum.to_list(101..203)
assert Enum.map(result, & &1.id) == page1_ids ++ page2_ids
assert_receive {:rest_hit, %{path: "/channels/2/messages", query_string: q1}}
assert q1 =~ "after=100"
@ -116,30 +116,6 @@ defmodule Dexcord.PaginationTest do
refute q2 =~ "before="
end
test "after: advances past each full descending page without overlapping messages" do
# Discord returns each after page newest first. Using its last wire item
# as the cursor would request overlapping messages on the next page.
for ids <- [200..101//-1, 300..201//-1, 350..301//-1] do
FakeRest.stub(
:get,
"/channels/4/messages",
FakeRest.resp(200, body: messages_json(ids))
)
end
ids = Api.message_history(4, after: 100) |> Enum.map(& &1.id)
assert ids == Enum.to_list(101..350)
assert length(Enum.uniq(ids)) == 250
for cursor <- [100, 200, 300] do
assert_receive {:rest_hit, %{path: "/channels/4/messages", query_string: query}}
assert URI.decode_query(query) == %{"after" => to_string(cursor), "limit" => "100"}
end
refute_receive {:rest_hit, %{path: "/channels/4/messages"}}, 50
end
test "limit: caps the total number of messages via Stream.take" do
full_page = Enum.to_list(300..201//-1)

View file

@ -49,12 +49,6 @@ defmodule Dexcord.FakeRest do
}
end
@doc """
A response marker that abruptly closes the TCP connection instead of
responding, so the client observes `%Mint.TransportError{reason: :closed}`.
"""
def close, do: :close
@doc false
# Called by the plug (in the Bandit connection process).
def handle_request(info), do: GenServer.call(__MODULE__, {:handle, info})
@ -134,24 +128,11 @@ defmodule Dexcord.FakeRest.Plug do
body: body
}
case Dexcord.FakeRest.handle_request(info) do
:close ->
abort_connection(conn)
response = Dexcord.FakeRest.handle_request(info)
response ->
conn
|> put_headers(response.headers)
|> send_resp(response.status, response.body)
end
end
# Slams the TCP connection shut without a response. The raise gets Bandit off
# this request; `Plug.BadRequestError`'s 400 status keeps it outside Bandit's
# default 500..599 exception-logging range, so the deliberate abort is silent.
defp abort_connection(conn) do
{Bandit.Adapter, adapter} = conn.adapter
ThousandIsland.Socket.close(adapter.transport.socket)
raise Plug.BadRequestError
conn
|> put_headers(response.headers)
|> send_resp(response.status, response.body)
end
defp put_headers(conn, headers) do