Skip to content

Conversation

@eabdelmoneim
Copy link
Contributor

@eabdelmoneim eabdelmoneim commented Jan 23, 2026

https://linear.app/thirdweb/issue/PRO-160/engine-v2-only-retrying-webhook-on-500-errors

Previously, only 5xx errors triggered webhook retries. This change adds 429 (Too Many Requests) and 403 (Forbidden) to the retry logic, treating them the same as server errors with exponential backoff.

This prevents webhook events from being lost when the destination endpoint rate-limits requests or returns transient auth errors.

Also fixes a pre-existing lint error (unused template literal).

Changes

How this PR will be tested

  • Open the dashboard and click X. Result: A modal should appear.
  • Call the /foo/bar API. Result: Returns 200 with "baz" in the response body.

Output

(Example: Screenshot/GIF for UI changes, cURL output for API changes)


PR-Codex overview

This PR enhances error handling in the send-webhook-worker.ts file by refining the conditions for retrying failed webhook requests. It updates the error message formatting and expands the range of HTTP status codes considered for retries.

Detailed summary

  • Updated the message format from using backticks to standard quotes.
  • Expanded the retry conditions to include HTTP status codes 429 (rate limit) and 403 (forbidden).
  • Introduced a new variable errorType to classify the error based on the status code.
  • Modified the log message to reflect the specific error type and status code.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Webhook delivery now retries on rate limit (HTTP 429) and forbidden (HTTP 403) responses, in addition to server errors, improving reliability.
    • Enhanced error logging with more descriptive messages identifying the specific failure reason.

✏️ Tip: You can customize this high-level summary in your review settings.

Previously, only 5xx errors triggered webhook retries. This change adds
429 (Too Many Requests) and 403 (Forbidden) to the retry logic, treating
them the same as server errors with exponential backoff.

This prevents webhook events from being lost when the destination
endpoint rate-limits requests or returns transient auth errors.

Also fixes a pre-existing lint error (unused template literal).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

Walkthrough

Extends webhook retry logic to include HTTP 429 (rate limit) and 403 (forbidden) errors alongside existing 5xx errors. Introduces error type mapping to categorize failure reasons and updates error logging to reflect the specific status code and reason. Minor log message normalization included.

Changes

Cohort / File(s) Summary
Webhook Retry Logic Enhancement
src/worker/tasks/send-webhook-worker.ts
Expands retry eligibility conditions to treat 429 and 403 as retryable errors (previously only 5xx). Adds errorType mapping to classify failures as rate-limited, forbidden, or server errors. Enriches error log message with specific HTTP status and reason. Normalizes log message for transaction-not-found case.
Dependency Update
package.json
Version or dependency updates (+10/-4 lines).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description includes relevant problem context and objectives but contains unfilled template sections (boilerplate placeholder text) and incomplete testing instructions. Complete the 'Changes' section with specific implementation details and replace placeholder testing steps with actual webhook retry test cases.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: extending webhook retry logic to handle 429 and 403 HTTP responses.
Linked Issues check ✅ Passed The code changes directly address PRO-160 by extending retry logic to treat HTTP 429 and 403 responses the same as 5xx errors with exponential backoff.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue: webhook retry logic expansion and a minor lint fix (unused template literal).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • INF-171: Entity not found: Issue - Could not find referenced Issue.

Comment @coderabbitai help to get the list of available commands and usage tips.

@nischitpra nischitpra merged commit c50bc44 into main Jan 24, 2026
8 checks passed
@nischitpra nischitpra deleted the fix/webhook-retry-on-429 branch January 24, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants