fix: Devin review bugs + prep @relayauth/server for publishing#9
fix: Devin review bugs + prep @relayauth/server for publishing#9khaliqgant wants to merge 1 commit intomainfrom
Conversation
Bug fixes: 1. audit-webhook-dispatcher: 4xx client errors now fail immediately instead of being caught and retried. Only 5xx/network errors retry. 2. identities.ts: sponsorChain now appends the NEW identity ID instead of duplicating the parent sub. Was [...chain, auth.claims.sub], now [...chain, id]. 3. create-identity.test.ts: Updated assertion to expect new identity ID in sponsorChain instead of the duplicated parent sub. Package prep: - Renamed from relayauth-server to @relayauth/server - Set version to 0.1.0 - Removed private: true - Added exports map for worker and durable-objects subpaths - Ready for npm publish
| if (error instanceof Error && error.message.includes("status 4")) { | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
🟡 HTTP 429 (Too Many Requests) incorrectly treated as permanent failure, preventing retries
The new catch block uses error.message.includes("status 4") to detect 4xx errors and fail immediately without retrying. This matches all 4xx status codes, including HTTP 429 (Too Many Requests), which is explicitly a retryable status code indicating rate limiting. Before this PR, a 429 response would be thrown at line 56, caught at line 60, stored as lastError, and then retried on the next loop iteration. After this PR, 429 triggers the includes("status 4") check and is re-thrown immediately, permanently losing the webhook delivery without any retry attempt. This is a semantic regression for a commonly encountered status code in webhook delivery scenarios.
| if (error instanceof Error && error.message.includes("status 4")) { | |
| throw error; | |
| } | |
| if (error instanceof Error && error.message.includes("status 4") && !error.message.includes("status 429")) { | |
| throw error; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
…cation) Cherry-picked from PR #9: 1. audit-webhook-dispatcher: 4xx client errors now fail immediately instead of being caught and retried 2. identities.ts: sponsorChain now appends the NEW identity ID instead of duplicating the parent sub 3. create-identity.test.ts: Updated assertion accordingly Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Combined into #10 — bug fixes cherry-picked into the server-package branch. |
Bug Fixes (from Devin review of AgentWorkforce/cloud#56)
1. Webhook 4xx retry bug
File:
packages/server/src/engine/audit-webhook-dispatcher.tsThe
throwfor 4xx status codes was caught by its owncatchblock, causing permanent client errors to be retried 3 times with exponential backoff. Now 4xx errors escape the retry loop immediately.2. sponsorChain duplication
File:
packages/server/src/routes/identities.ts:455When creating a child identity, the sponsorChain was built as
[...parentChain, parent.sub]which duplicates the parent. Fixed to[...parentChain, newIdentityId].3. Test assertion
File:
packages/server/src/__tests__/create-identity.test.tsUpdated the assertion to expect the new identity's ID (not the duplicated parent sub).
Package Prep
relayauth-server→@relayauth/server0.1.0private: true.,./worker,./durable-objects)npm publishafter mergeNext Steps
Once published,
AgentWorkforce/cloudPR #56 will updatepackages/relayauth/to import from@relayauth/serverinstead of using relative path re-exports.