fix(telemetry): a rejection Discord answered outranks our own budget #729

Merged
coilyco-ops merged 1 commit from fix/a-rest-failure-keeps-its-status into main 2026-08-13 20:59:44 +00:00
Member

Slice of issue 292. The delivery-failure classifier that issue depends on discards Discord's verdict whenever a context error travels beside it.

Measured

rest alone      -> [discord_failure=rest_error discord_status=403 discord_code=50013]
join(rest,ctx)  -> [discord_failure=abandoned]
join(ctx,rest)  -> [discord_failure=abandoned]

A 403 with code 50013, a missing permission, reports as abandoned and loses its status and code.

Why that is the common path, not an exotic one

// agent.go:1142
if err := a.sendReply(turnCtx, turn, reply); err != nil {
    return errors.Join(err, a.reportUndelivered(turnCtx, turn))
}

runSerialized returns that join and discord.turn.failed classifies it. reportUndelivered sends a notice on a turn whose context has often just expired, so the join carrying a context error is the normal shape of a failed delivery.

discordFailureAttrs returns on its first match and the context check was first, per issue 648. That precedence is correct when the error is only a context error. When both are present Discord did answer, and abandoned says the opposite.

The existing test already claimed this

TestARestErrorIsStillClassifiedByItsStatus carries the comment:

A rejection Discord did answer outranks nothing here, because a REST error carrying a cancelled context is still Discord's verdict.

It passes a bare rejection and never a joined one, so it asserted the sentence it did not test. Strengthened rather than replaced.

The change

Look for the rejection first; fall back to the context check and then the catch-all. 648's case is untouched: a pure context error, wrapped or joined with a plain error, still reads abandoned.

Acceptance

  • A rejection joined with a context error, in either order, reports rest_error and keeps its status and code.
  • A pure context error still reports abandoned.
  • A transport failure still reports no_response.
  • A rejection with no response body classifies without inventing a status.
  • Each row fails under a mutation restoring the original precedence, in a run where the package compiles.

Next owner

Engineer. Closed by the pull request that lands the change.

ward exec gate PASS on all six steps. Mutation: restoring the original precedence fails both new rows in a run where the package compiles.

closes #727

Slice of [issue 292](https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/292). The delivery-failure classifier that issue depends on discards Discord's verdict whenever a context error travels beside it. ## Measured ``` rest alone -> [discord_failure=rest_error discord_status=403 discord_code=50013] join(rest,ctx) -> [discord_failure=abandoned] join(ctx,rest) -> [discord_failure=abandoned] ``` A 403 with code 50013, a missing permission, reports as `abandoned` and loses its status and code. ## Why that is the common path, not an exotic one ```go // agent.go:1142 if err := a.sendReply(turnCtx, turn, reply); err != nil { return errors.Join(err, a.reportUndelivered(turnCtx, turn)) } ``` `runSerialized` returns that join and `discord.turn.failed` classifies it. `reportUndelivered` sends a notice on a turn whose context has often just expired, so the join carrying a context error is the normal shape of a failed delivery. `discordFailureAttrs` returns on its first match and the context check was first, per [issue 648](https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/648). That precedence is correct when the error is **only** a context error. When both are present Discord did answer, and `abandoned` says the opposite. ## The existing test already claimed this `TestARestErrorIsStillClassifiedByItsStatus` carries the comment: > A rejection Discord did answer outranks nothing here, because a REST error carrying a cancelled context is still Discord's verdict. It passes a **bare** rejection and never a joined one, so it asserted the sentence it did not test. Strengthened rather than replaced. ## The change Look for the rejection first; fall back to the context check and then the catch-all. 648's case is untouched: a pure context error, wrapped or joined with a plain error, still reads `abandoned`. ## Acceptance - A rejection joined with a context error, in either order, reports `rest_error` and keeps its status and code. - A pure context error still reports `abandoned`. - A transport failure still reports `no_response`. - A rejection with no response body classifies without inventing a status. - Each row fails under a mutation restoring the original precedence, in a run where the package compiles. ## Next owner Engineer. Closed by the pull request that lands the change. `ward exec gate` PASS on all six steps. Mutation: restoring the original precedence fails both new rows in a run where the package compiles. closes #727
fix(telemetry): a rejection Discord answered outranks our own budget
All checks were successful
ci / image-build (pull_request) Successful in 23s
ci / test (pull_request) Successful in 42s
ci / publish-echo-image (pull_request) Has been skipped
ci / publish-observed (pull_request) Has been skipped
6db80fd071
A 403 with code 50013 reported as abandoned, losing its status and code,
whenever a context error travelled beside it. That is the normal shape of a
failed delivery: the turn returns errors.Join(send, notice) and the notice runs
on a context the send has often just exhausted.

648's precedence is right when the error is only a context error. When both are
present Discord did answer, so the rejection is the true statement.

TestARestErrorIsStillClassifiedByItsStatus already claimed this in its comment
and passed only a bare rejection, so it asserted what it did not test.
Strengthened rather than replaced.

closes #727

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo!729
No description provided.