-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add retry mechanism for failed requests #3213
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReplaces positional error handler usage across many cron routes with a new cron-specific wrapper, refactors the central error handler to accept an object and detect QStash callbacks / transient Prisma errors, adds queueFailedRequestForRetry (QStash retry publisher), and updates non-cron routes/auth call sites to the new error-handler parameter shape. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as App Route
participant DB as Prisma
participant Error as handleAndReturnErrorResponse
participant Queue as QStash
participant Logger
Client->>API: Send request
API->>DB: Perform DB operation
DB-->>API: Throws transient DB error
API->>Error: catch -> handleAndReturnErrorResponse({ error, requestHeaders, responseHeaders })
Error->>Error: map error via handleApiError()
Error->>Error: is QStash callback? (check upstash-signature)
alt QStash callback & transient DB error
Error->>Queue: publish retry job (jitter + backoff, 5 retries)
Error->>Logger: log queued retry
Error-->>API: return JSON error with retry headers/status
else Standard request or non-retryable
Error-->>API: return mapped JSON error & status
end
API-->>Client: Error response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-12-15T16:45:51.667ZApplied to files:
📚 Learning: 2025-12-09T12:54:41.818ZApplied to files:
🧬 Code graph analysis (1)apps/web/app/(ee)/api/cron/payouts/payout-failed/route.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/app/api/analytics/dashboard/route.ts (1)
162-171: FixwaitUntilusage; error handler change is fineThe switch to
handleAndReturnErrorResponse({ error })aligns with the function's options‑object signature. However, this line:waitUntil(await redis.set(cacheKey, response, { ex: 60 }));awaits the Redis write before passing it to
waitUntil, sowaitUntilreceives the resolved value ("OK") rather than the Promise. You likely want:waitUntil(redis.set(cacheKey, response, { ex: 60 }));so the cache write runs as deferred work as intended.
apps/web/app/(ee)/api/cron/network/calculate-program-similarities/route.ts (1)
202-211: QStash URL is missing the/networksegmentThe route is located at
api/cron/network/calculate-program-similarities, but thepublishJSONcall at line 203 uses/api/cron/calculate-program-similarities, which is a non-existent route. This will cause scheduled follow-up batches to fail.Update to:
- url: `${APP_DOMAIN_WITH_NGROK}/api/cron/calculate-program-similarities`, + url: `${APP_DOMAIN_WITH_NGROK}/api/cron/network/calculate-program-similarities`,apps/web/app/(ee)/api/cron/utils.ts (1)
1-33:⚠️ Missing retry mechanism contradicts PR objectives.The PR is titled "Add retry mechanism for failed requests" and the AI summary claims this introduces "Prisma-based transient error detection for retry decisions" and "a QStash-aware retry queue utility for failed requests." However,
handleCronErrorResponseonly formats error responses—it doesn't detect transient errors, queue retries, or implement any retry logic.If the retry mechanism exists in other files not included in this review, please clarify. Otherwise, this appears to be a straightforward error handling refactoring rather than a retry implementation.
♻️ Duplicate comments (4)
apps/web/app/(ee)/api/cron/workspaces/delete/route.ts (1)
109-109: Same QStash retry logic concern as other routes.This route also uses
verifyQstashSignature(line 19) but switches tohandleCronErrorResponse. Please ensure the QStash retry mechanism is preserved in the new error handler.apps/web/app/(ee)/api/cron/cleanup/expired-tokens/route.ts (1)
67-67: Same QStash retry logic concern as other routes.This route uses
verifyQstashSignature(line 19) but switches tohandleCronErrorResponse. Ensure QStash retry logic is preserved.apps/web/app/(ee)/api/cron/payouts/process/route.ts (1)
83-83: Same QStash retry logic concern as other routes.This route uses
verifyQstashSignature(line 31) but switches tohandleCronErrorResponse. Ensure QStash retry logic is preserved.apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts (1)
10-10: Cron utils integration is consistent with other routesUsing
{ handleCronErrorResponse, logAndRespond }here matches the pattern in other cron routes; same note as inimport/csvabout confirming whether the simpler cron error handler should influence QStash retry behavior.Also applies to: 230-238
🧹 Nitpick comments (19)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/payouts/payout-stats.tsx (1)
140-141: Recommended refactor: Remove redundant " USD" suffix.The
currencyFormatterfunction already formats amounts with a currency symbol (e.g.,$1,234.56). Appending" USD"results in redundant currency indication (e.g.,$1,234.56 USD). Consider passing the currency option explicitly if needed, or removing the manual suffix.As per the currencyFormatter documentation: the function uses
Intl.NumberFormatwith the currency style and defaults to USD when no explicit currency is provided.Example refactor:
- {currencyFormatter(eligiblePendingPayouts?.amount ?? 0, {}) + - " USD"} + {currencyFormatter(eligiblePendingPayouts?.amount ?? 0, { currency: "USD" })}You may defer this cleanup to a future PR if prioritizing other changes.
Also applies to: 198-198
apps/web/lib/auth/partner.ts (1)
171-174: Consider passing requestHeaders for QStash retry support.The error handling update is correct and properly passes
responseHeaders. However, since this function has access toreq, consider also passingrequestHeadersto enable QStash-aware retry logic when partner routes are called via QStash callbacks.Apply this diff to add requestHeaders support:
} catch (error) { return handleAndReturnErrorResponse({ error, responseHeaders, + requestHeaders: req.headers, }); }apps/web/ui/modals/domain-auto-renewal-modal.tsx (1)
96-98: Minor formatting change unrelated to PR objective.The text reflow here appears to be an unrelated formatting change. While it has no functional impact, it's worth noting that this cosmetic change doesn't align with the PR's retry mechanism objective.
apps/web/app/api/oauth/userinfo/route.ts (1)
73-77: Good improvement with consistent error naming.The change correctly updates error handling to use the new object-based signature and improves consistency by renaming
etoerror. The CORS headers are properly passed asresponseHeaders.For completeness with the retry mechanism, consider also passing
requestHeaders:} catch (error) { return handleAndReturnErrorResponse({ error, responseHeaders: CORS_HEADERS, + requestHeaders: req.headers, }); }apps/web/ui/customers/customer-sales-table.tsx (1)
61-61: Cosmetic simplification - unrelated to PR objective.The cell renderer has been simplified to a concise single-line form. While this improves readability, it's unrelated to the retry mechanism that is the focus of this PR.
apps/web/ui/customers/customer-partner-earnings-table.tsx (1)
36-36: Cosmetic simplification - unrelated to PR objective.Both the "Sale Amount" and "Commission" cell renderers have been simplified to concise single-line forms. While this improves code consistency with
customer-sales-table.tsx, these formatting changes are unrelated to the retry mechanism objective of this PR.Also applies to: 41-41
apps/web/lib/api/queue-failed-request-for-retry.ts (2)
44-53: Consider wrapping QStash publish in try-catch.If
qstash.publishJSONfails (network issues, QStash unavailable), the error will propagate and could mask the original error or cause unexpected behavior in the caller. Since this is a best-effort retry mechanism, failures should be logged but not block the error response.- const response = await qstash.publishJSON({ - url, - method, - body: await parseRequestBody(errorReq), - headers: { - ...Object.fromEntries(errorReq.headers.entries()), - }, - delay: "10s", - retries: 5, - }); - - if (response.messageId) { + try { + const response = await qstash.publishJSON({ + url, + method, + body: await parseRequestBody(errorReq), + headers: { + ...Object.fromEntries(errorReq.headers.entries()), + }, + delay: "10s", + retries: 5, + }); + + if (response.messageId) { + console.log("Request queued for retry", { + method, + url, + messageId: response.messageId, + }); + + logger.info("request.retry.queued", { + url, + method, + messageId: response.messageId, + }); + + await logger.flush(); + } + } catch (e) { + console.error("Failed to queue request for retry", { method, url, error: e }); + logger.error("request.retry.queue_failed", { url, method, error: e }); + await logger.flush(); + }
48-50: Consider filtering sensitive headers before forwarding.Forwarding all headers to QStash may inadvertently include sensitive information beyond the API key (e.g., cookies, internal tracing headers). Consider explicitly allowlisting headers that are necessary for retry.
+const ALLOWED_RETRY_HEADERS = [ + "authorization", + "content-type", + "x-api-version", + // Add other necessary headers +]; + +function filterHeaders(headers: Headers): Record<string, string> { + const filtered: Record<string, string> = {}; + for (const key of ALLOWED_RETRY_HEADERS) { + const value = headers.get(key); + if (value) filtered[key] = value; + } + return filtered; +}Then use
headers: filterHeaders(errorReq.headers)instead of spreading all entries.apps/web/lib/api/errors.ts (1)
59-63: Consider exportingErrorHandlerOptionsfor caller type safety.The interface is used in the public
handleAndReturnErrorResponsesignature but isn't exported. Callers may need to import this type to properly type their error handling code.-interface ErrorHandlerOptions { +export interface ErrorHandlerOptions { error: unknown; responseHeaders?: Headers; requestHeaders?: Headers; }apps/web/app/(ee)/api/cron/import/csv/route.ts (1)
24-24: Confirm cron retry behavior with new error handlerThis route now uses
handleCronErrorResponse({ error }), which (perapps/web/app/(ee)/api/cron/utils.ts) just maps the error to{ error, status }and doesn’t look at the request headers or setUpstash-NonRetryable-ErrorlikehandleAndReturnErrorResponsedoes. If this endpoint is invoked by QStash Cron, that may change how permanent vs transient errors are retried. Worth confirming whether cron failures should always be retried, or if you also want the non‑retryable header logic here.Also applies to: 124-132
apps/web/app/api/links/metatags/route.ts (1)
34-45: Preserve CORS headers on error responsesWrapping the error in
{ error }matches the newhandleAndReturnErrorResponsesignature. To keep CORS behavior consistent, consider:} catch (error) { return handleAndReturnErrorResponse({ error, responseHeaders: corsHeaders, }); }so callers receive the same CORS headers on failures as on success.
apps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.ts (1)
66-73: Standardized cron error response; consider tightening log typingSwitching the catch path to
handleCronErrorResponse({ error })keeps this route aligned with the new cron-wide error handling and retry semantics. To make the log call a bit safer, consider guardingerror.message(as you already do in other routes) with aninstanceof Errorcheck orString(error).apps/web/app/(ee)/api/cron/bounties/notify-partners/route.ts (1)
211-218: Error response now aligned with cron-wide behaviorDelegating to
handleCronErrorResponse({ error })after logging ensures a uniform JSON+status shape for cron failures, which is important for the new retry flow. As a minor improvement, you could mirror theinstanceof Errorguard used in other routes to avoid relying onerror.messagewhenerrormight not be anError.apps/web/app/(ee)/api/cron/import/tolt/route.ts (1)
47-49: Consistent cron error response; consider adding a logReturning
handleCronErrorResponse({ error })standardizes the failure shape for this import job. If you want parity with other cron routes, consider logging the error (and perhapspayload.action) before returning, to make diagnosing failed Tolt imports easier.apps/web/app/(ee)/api/cron/groups/sync-utm/route.ts (1)
145-151: Unified cron error response; optional hardening of logThe switch to
handleCronErrorResponse({ error })after logging ensures failures from this sync job surface through the standardized cron error pipeline. As a minor improvement, you may want to mirror theinstanceof Errorguard pattern from other routes soerror.messageaccess is always safe.apps/web/app/(ee)/api/cron/import/short/route.ts (1)
61-68: Cron error response integration looks good; minor logging nitThe outer catch now logs and then returns
handleCronErrorResponse({ error }), which is aligned with the new cron error model and retry behavior. If you want to be defensive, you can guarderror.messagein the log (and in the inner catch when building theDubApiErrormessage) in case a non-Error slips through.apps/web/app/(ee)/api/cron/payouts/balance-available/route.ts (1)
166-173: Standardized cron error handling; optional log robustnessUsing
handleCronErrorResponse({ error })after logging ensures payout failures are reported through the same cron error pipeline that drives retries and monitoring. As a small hardening step, you might guarderror.messagein the log with aninstanceof Errorcheck, to be resilient to non-Error throws.apps/web/app/(ee)/api/cron/fx-rates/route.ts (1)
5-5: Cron error handler integration is correct; consider minor type-safety tweakThe import path and
handleCronErrorResponse({ error })usage look correct and align this route with the new cron error handling.If your TS config uses
useUnknownInCatchVariables, you may eventually want to narrowerrorbefore interpolatingerror.messagein the log (e.g.,error instanceof Error ? error.message : String(error)), but that’s an optional clean-up and not introduced by this PR.Also applies to: 52-52
apps/web/app/(ee)/api/cron/payouts/process/updates/route.ts (1)
9-9: Error routing wired correctly; log message could be clearerThe import path and
handleCronErrorResponse({ error })usage look correct and hook this updates worker into the shared cron error / retry handling.Optionally, you might later tweak the log message (
Error sending Stripe payout) to more clearly reference thepayouts/process/updatesjob so logs are easier to grep by route, but that’s not blocking.Also applies to: 180-180
apps/web/app/(ee)/api/cron/cleanup/demo-embed-partners/route.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/app/(ee)/api/cron/import/rebrandly/route.ts (1)
87-94: Cron error handling correctly delegates tohandleCronErrorResponse(optional hardening)Routing the outer catch through
handleCronErrorResponse({ error })standardizes cron responses and aligns with the cron-specific error pattern described in the learnings (no QStash callback detection or Upstash non‑retry headers in cron routes). Based on learnings, this is the intended behavior.If your
tsconfigusesuseUnknownInCatchVariables, you might optionally normalize the error before accessing.message:- } catch (error) { - await log({ - message: `Error importing Rebrandly links: ${error.message}`, - type: "cron", - }); - - return handleCronErrorResponse({ error }); - } + } catch (error) { + const err = error instanceof Error ? error : new Error(String(error)); + + await log({ + message: `Error importing Rebrandly links: ${err.message}`, + type: "cron", + }); + + return handleCronErrorResponse({ error: err }); + }Purely a safety/clarity tweak; behavior is otherwise sound.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(ee)/api/cron/cleanup/demo-embed-partners/route.ts(2 hunks)apps/web/app/(ee)/api/cron/import/rebrandly/route.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(ee)/api/cron/cleanup/demo-embed-partners/route.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:45:51.667Z
Learning: In the Dub codebase, cron endpoints under apps/web/app/(ee)/api/cron/ use handleCronErrorResponse for error handling, which intentionally does NOT detect QStash callbacks or set Upstash-NonRetryable-Error headers. This allows QStash to retry all cron job errors using its native retry mechanism. The selective retry logic (queueFailedRequestForRetry) is only used for specific user-facing API endpoints like /api/track/lead, /api/track/sale, and /api/links to retry only transient Prisma database errors.
📚 Learning: 2025-12-09T12:54:41.818Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3207
File: apps/web/lib/cron/with-cron.ts:27-56
Timestamp: 2025-12-09T12:54:41.818Z
Learning: In `apps/web/lib/cron/with-cron.ts`, the `withCron` wrapper extracts the request body once and provides it to handlers via the `rawBody` parameter. Handlers should use this `rawBody` string parameter (e.g., `JSON.parse(rawBody)`) rather than reading from the Request object via `req.json()` or `req.text()`.
Applied to files:
apps/web/app/(ee)/api/cron/import/rebrandly/route.ts
📚 Learning: 2025-12-15T16:45:51.667Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:45:51.667Z
Learning: In cron endpoints under apps/web/app/(ee)/api/cron, continue using handleCronErrorResponse for error handling. Do not detect QStash callbacks or set Upstash-NonRetryable-Error headers in these cron routes, so QStash can retry cron errors via its native retry mechanism. The existing queueFailedRequestForRetry logic should remain limited to specific user-facing API endpoints (e.g., /api/track/lead, /api/track/sale, /api/links) to retry only transient Prisma/database errors. This pattern should apply to all cron endpoints under the cron directory in this codebase.
Applied to files:
apps/web/app/(ee)/api/cron/import/rebrandly/route.ts
📚 Learning: 2025-05-29T09:49:19.604Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2433
File: apps/web/ui/modals/add-payment-method-modal.tsx:60-62
Timestamp: 2025-05-29T09:49:19.604Z
Learning: The `/api/workspaces/${slug}/billing/payment-methods` POST endpoint in the billing API returns either an error (handled by response.ok check) or a response object containing a `url` property for successful requests.
Applied to files:
apps/web/app/(ee)/api/cron/import/rebrandly/route.ts
📚 Learning: 2025-06-18T20:26:25.177Z
Learnt from: TWilson023
Repo: dubinc/dub PR: 2538
File: apps/web/ui/partners/overview/blocks/commissions-block.tsx:16-27
Timestamp: 2025-06-18T20:26:25.177Z
Learning: In the Dub codebase, components that use workspace data (workspaceId, defaultProgramId) are wrapped in `WorkspaceAuth` which ensures these values are always available, making non-null assertions safe. This is acknowledged as a common pattern in their codebase, though not ideal.
Applied to files:
apps/web/app/(ee)/api/cron/import/rebrandly/route.ts
🧬 Code graph analysis (1)
apps/web/app/(ee)/api/cron/import/rebrandly/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
apps/web/app/(ee)/api/cron/import/rebrandly/route.ts (2)
1-7: Error-handling imports are consistent and necessaryKeeping
DubApiErrorfor explicitbad_requestthrows and addinghandleCronErrorResponsefor the top-level catch cleanly separates domain errors from cron response formatting. This matches the shared cron-utils pattern; no issues here.
82-85: Workspace error message formatting fix looks goodThe updated message string:
message: `Workspace: ${workspace?.slug || workspaceId}. Error: ${error.message}`,removes the stray
$and reads clearly while preserving the workspace context and underlying error. All good here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.tsapps/web/app/(ee)/api/cron/campaigns/broadcast/route.tsapps/web/app/(ee)/api/cron/cleanup/rejected-applications/route.tsapps/web/app/(ee)/api/cron/links/export/route.tsapps/web/app/(ee)/api/cron/merge-partner-accounts/route.tsapps/web/app/(ee)/api/cron/network/calculate-program-similarities/route.tsapps/web/app/(ee)/api/cron/payouts/aggregate-due-commissions/route.tsapps/web/app/(ee)/api/cron/payouts/payout-paid/route.tsapps/web/app/(ee)/api/cron/payouts/process/route.tsapps/web/app/(ee)/api/cron/payouts/reminders/partners/route.tsapps/web/app/(ee)/api/track/click/route.tsapps/web/app/api/analytics/dashboard/route.tsapps/web/lib/auth/workspace.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/web/app/(ee)/api/cron/payouts/aggregate-due-commissions/route.ts
- apps/web/app/(ee)/api/cron/cleanup/rejected-applications/route.ts
- apps/web/app/(ee)/api/track/click/route.ts
- apps/web/app/(ee)/api/cron/merge-partner-accounts/route.ts
- apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts
- apps/web/app/(ee)/api/cron/network/calculate-program-similarities/route.ts
- apps/web/app/(ee)/api/cron/payouts/process/route.ts
- apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:46:01.529Z
Learning: In the Dub codebase, cron endpoints under apps/web/app/(ee)/api/cron/ use handleCronErrorResponse for error handling, which intentionally does NOT detect QStash callbacks or set Upstash-NonRetryable-Error headers. This allows QStash to retry all cron job errors using its native retry mechanism. The selective retry logic (queueFailedRequestForRetry) is only used for specific user-facing API endpoints like /api/track/lead, /api/track/sale, and /api/links to retry only transient Prisma database errors.
📚 Learning: 2025-12-15T16:45:51.667Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:45:51.667Z
Learning: In cron endpoints under apps/web/app/(ee)/api/cron, continue using handleCronErrorResponse for error handling. Do not detect QStash callbacks or set Upstash-NonRetryable-Error headers in these cron routes, so QStash can retry cron errors via its native retry mechanism. The existing queueFailedRequestForRetry logic should remain limited to specific user-facing API endpoints (e.g., /api/track/lead, /api/track/sale, /api/links) to retry only transient Prisma/database errors. This pattern should apply to all cron endpoints under the cron directory in this codebase.
Applied to files:
apps/web/app/(ee)/api/cron/payouts/payout-paid/route.tsapps/web/app/(ee)/api/cron/payouts/reminders/partners/route.tsapps/web/app/(ee)/api/cron/links/export/route.ts
📚 Learning: 2025-12-09T12:54:41.818Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3207
File: apps/web/lib/cron/with-cron.ts:27-56
Timestamp: 2025-12-09T12:54:41.818Z
Learning: In `apps/web/lib/cron/with-cron.ts`, the `withCron` wrapper extracts the request body once and provides it to handlers via the `rawBody` parameter. Handlers should use this `rawBody` string parameter (e.g., `JSON.parse(rawBody)`) rather than reading from the Request object via `req.json()` or `req.text()`.
Applied to files:
apps/web/app/(ee)/api/cron/payouts/payout-paid/route.tsapps/web/app/(ee)/api/cron/payouts/reminders/partners/route.tsapps/web/app/(ee)/api/cron/links/export/route.ts
📚 Learning: 2025-10-02T22:46:22.739Z
Learnt from: steven-tey
Repo: dubinc/dub PR: 2924
File: apps/web/lib/api/conversions/track-lead.ts:7-7
Timestamp: 2025-10-02T22:46:22.739Z
Learning: In apps/web/lib/api/conversions/track-lead.ts, lead events are cached in Redis for 5 minutes (keys: `leadCache:${customer.id}` and `leadCache:${customer.id}:${stringifiedEventName}`) to provide immediate data availability while Tinybird ingestion happens asynchronously. This caching pattern allows for async-only recording without breaking "wait" mode semantics.
Applied to files:
apps/web/lib/auth/workspace.ts
🧬 Code graph analysis (5)
apps/web/app/(ee)/api/cron/payouts/payout-paid/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/api/analytics/dashboard/route.ts (1)
apps/web/lib/api/errors.ts (1)
handleAndReturnErrorResponse(169-201)
apps/web/app/(ee)/api/cron/payouts/reminders/partners/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/lib/auth/workspace.ts (4)
apps/web/lib/edge-config/get-feature-flags.ts (1)
getFeatureFlags(7-54)apps/web/lib/api/queue-failed-request-for-retry.ts (1)
queueFailedRequestForRetry(15-70)apps/web/lib/tinybird/log-conversion-events.ts (1)
logConversionEvent(13-16)apps/web/lib/api/errors.ts (1)
handleAndReturnErrorResponse(169-201)
apps/web/app/(ee)/api/cron/links/export/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
apps/web/app/(ee)/api/cron/payouts/payout-paid/route.ts (2)
7-7: LGTM! Correct import for cron-specific error handling.The import of
handleCronErrorResponseis appropriate for this cron endpoint and aligns with the established pattern for cron error handling in the codebase.Based on learnings, cron endpoints under
apps/web/app/(ee)/api/cron/should usehandleCronErrorResponseto allow QStash to retry errors via its native retry mechanism.
79-86: LGTM! Proper cron error handling with retry support.The error handling correctly uses
handleCronErrorResponse({ error })which aligns with the cron endpoint pattern. The error is appropriately logged before returning the response, and the new handler allows QStash to retry failed requests using its native retry mechanism.Based on learnings, this pattern ensures QStash can retry all cron job errors without selective filtering, which is the intended behavior for cron endpoints in this codebase.
apps/web/app/(ee)/api/cron/payouts/reminders/partners/route.ts (2)
9-9: LGTM! Correct error handler for cron endpoints.The import correctly switches to
handleCronErrorResponsefrom the cron utils module. Based on learnings, this cron-specific error handler intentionally does NOT set Upstash-NonRetryable-Error headers, allowing QStash to retry all cron job errors using its native retry mechanism.
207-207: LGTM! Proper call signature for cron error handler.The error handling correctly uses the new
handleCronErrorResponse({ error })call signature, which aligns with the cron-specific error handling pattern established across other cron routes in this PR.apps/web/app/(ee)/api/cron/links/export/route.ts (2)
18-18: LGTM! Import updated correctly for cron error handling.The import change from
handleAndReturnErrorResponsetohandleCronErrorResponsealigns with the established pattern for cron endpoints in this codebase, ensuring QStash can utilize its native retry mechanism for all cron errors.Based on learnings, this is the correct error handler for cron routes.
147-154: LGTM! Error handling updated correctly.The error handling logic is correct:
- Error logging is preserved (lines 148-151)
- The call to
handleCronErrorResponse({ error })properly returns aNextResponsewith the appropriate error structure- This pattern ensures QStash can retry cron job failures using its native retry mechanism without selective filtering
Based on learnings, this is the correct approach for cron endpoints.
apps/web/lib/auth/workspace.ts (5)
9-9: LGTM: Import added for retry mechanism.The import of
queueFailedRequestForRetryis correctly added to support the new retry logic for failed requests.
430-430: LGTM: Improved immutability.Using
constinstead ofletis appropriate sinceflagsis not reassigned.
481-493: LGTM: Conversion logging refined for retry endpoints.The conversion event logging is now appropriately scoped to only the endpoints that utilize the retry mechanism (
/api/track/leadand/api/track/sale), which aligns with the selective retry strategy mentioned in the learnings. Theworkspaceexistence check prevents potential errors when the failure occurs before workspace resolution.Based on learnings, this selective logging approach is intentional for debugging retry behavior on specific user-facing API endpoints.
497-501: LGTM: Enhanced error handler signature for QStash integration.The updated signature allows
handleAndReturnErrorResponseto access request headers and detect QStash callbacks via theupstash-signatureheader. This enables proper retry control by setting theUpstash-NonRetryable-Errorheader for non-recoverable errors while allowing retries for transient Prisma database errors.
475-479: Well-designed retry mechanism with internal filtering.The retry logic is correctly integrated into the error handler. The
queueFailedRequestForRetryfunction internally filters which endpoints are retryable usingRETRYABLE_RULES(configured for/api/track/lead,/api/track/sale, and/api/links) and only queues retries for transient Prisma database errors, preventing unnecessary retries for other error types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/lib/api/errors.ts (1)
169-198: Solid QStash retry logic implementation.The selective retry mechanism correctly allows QStash retries only for
PrismaClientUnknownRequestError(transient DB errors). Based on learnings, this is intentionally applied to specific user-facing API endpoints while cron jobs use a separate handler that allows all retries.One consideration:
PrismaClientInitializationErrorcould also indicate transient issues (e.g., connection pool exhaustion). If you encounter such errors in production, you may want to expand the retry eligibility.🔎 Optional: Expand transient error detection if needed in the future
- const shouldRetry = error instanceof Prisma.PrismaClientUnknownRequestError; + const shouldRetry = + error instanceof Prisma.PrismaClientUnknownRequestError || + error instanceof Prisma.PrismaClientInitializationError;apps/web/lib/api/queue-failed-request-for-retry.ts (1)
27-28: Consider explicit method validation for defensive coding.The method is cast to
HTTPMethodswithout validation. While theisRetryablecheck on line 38 indirectly validates the method (since only POST routes are currently configured), adding explicit validation would make the code more defensive against future changes.🔎 Suggested validation approach
const method = errorReq.method as HTTPMethods; + const supportedMethods: HTTPMethods[] = ['GET', 'POST', 'PUT', 'DELETE', 'PATCH']; + if (!supportedMethods.includes(method)) { + console.warn(`Unsupported HTTP method for retry: ${method}`); + return; + } const isRetryable = RETRYABLE_ROUTES[method]?.has(pathname) ?? false;This provides an explicit check and clear logging if an unsupported method is encountered.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/lib/api/errors.tsapps/web/lib/api/queue-failed-request-for-retry.tsapps/web/lib/auth/workspace.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/lib/auth/workspace.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:46:01.529Z
Learning: In the Dub codebase, cron endpoints under apps/web/app/(ee)/api/cron/ use handleCronErrorResponse for error handling, which intentionally does NOT detect QStash callbacks or set Upstash-NonRetryable-Error headers. This allows QStash to retry all cron job errors using its native retry mechanism. The selective retry logic (queueFailedRequestForRetry) is only used for specific user-facing API endpoints like /api/track/lead, /api/track/sale, and /api/links to retry only transient Prisma database errors.
📚 Learning: 2025-12-15T16:46:01.529Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:46:01.529Z
Learning: In the Dub codebase, cron endpoints under apps/web/app/(ee)/api/cron/ use handleCronErrorResponse for error handling, which intentionally does NOT detect QStash callbacks or set Upstash-NonRetryable-Error headers. This allows QStash to retry all cron job errors using its native retry mechanism. The selective retry logic (queueFailedRequestForRetry) is only used for specific user-facing API endpoints like /api/track/lead, /api/track/sale, and /api/links to retry only transient Prisma database errors.
Applied to files:
apps/web/lib/api/queue-failed-request-for-retry.ts
📚 Learning: 2025-10-17T08:18:19.278Z
Learnt from: devkiran
Repo: dubinc/dub PR: 0
File: :0-0
Timestamp: 2025-10-17T08:18:19.278Z
Learning: In the apps/web codebase, `@/lib/zod` should only be used for places that need OpenAPI extended zod schema. All other places should import from the standard `zod` package directly using `import { z } from "zod"`.
Applied to files:
apps/web/lib/api/errors.ts
🧬 Code graph analysis (1)
apps/web/lib/api/errors.ts (1)
packages/prisma/client.ts (1)
Prisma(30-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
apps/web/lib/api/errors.ts (3)
2-2: LGTM!Import is correctly added to support the
Prisma.PrismaClientUnknownRequestErrortype check for transient database error detection.
59-63: LGTM!Clean interface design that groups related parameters. Optional headers provide flexibility for different call sites.
114-167: LGTM!Well-structured extraction of error-to-response mapping logic. This enables reuse across different response handlers (e.g.,
handleAndReturnErrorResponsefor API routes andhandleCronErrorResponsefor cron jobs). Error handling covers all expected cases with appropriate fallback.apps/web/lib/api/queue-failed-request-for-retry.ts (2)
26-26: No action needed.APP_DOMAIN_WITH_NGROKis correctly configured to use the production domain in production environments—the misleading variable name notwithstanding. The constant conditionally returnshttps://app.${NEXT_PUBLIC_APP_DOMAIN}whenNEXT_PUBLIC_VERCEL_ENV === "production"and only falls back to ngrok domains in development or preview environments.Likely an incorrect or invalid review comment.
24-55: The request body is properly preserved. The originalreqpassed to this function is intentionally kept unconsumed—only the cloned request is used by handlers (line 463 inworkspace.ts). When an error occurs, the error handler invokes this function with the original, unconsumed request, ensuring the body is available for retry.Likely an incorrect or invalid review comment.
| const response = await qstash.publishJSON({ | ||
| url, | ||
| method, | ||
| body: await parseRequestBody(errorReq), | ||
| headers: Object.fromEntries(errorReq.headers.entries()), | ||
| retries: 5, | ||
| retryDelay, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filter sensitive headers before forwarding to QStash.
Forwarding all headers without filtering (line 56) could expose sensitive information like authorization tokens, cookies, or internal routing headers to QStash. While some headers are necessary for successful retry (e.g., Content-Type, Accept), sensitive headers should be filtered.
🔎 Suggested approach for filtering headers
Consider creating a header allowlist or blocklist:
+ const SENSITIVE_HEADERS = new Set([
+ 'authorization',
+ 'cookie',
+ 'set-cookie',
+ 'x-api-key',
+ 'proxy-authorization',
+ ]);
+
+ const sanitizedHeaders = Object.fromEntries(
+ Array.from(errorReq.headers.entries()).filter(
+ ([key]) => !SENSITIVE_HEADERS.has(key.toLowerCase())
+ )
+ );
const response = await qstash.publishJSON({
url,
method,
body: await parseRequestBody(errorReq),
- headers: Object.fromEntries(errorReq.headers.entries()),
+ headers: sanitizedHeaders,
retries: 5,
retryDelay,
});Alternatively, if the API key is the primary authentication mechanism and is already captured in the apiKey parameter, ensure it's re-injected during retry rather than relying on forwarded headers.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const response = await qstash.publishJSON({ | |
| url, | |
| method, | |
| body: await parseRequestBody(errorReq), | |
| headers: Object.fromEntries(errorReq.headers.entries()), | |
| retries: 5, | |
| retryDelay, | |
| }); | |
| const SENSITIVE_HEADERS = new Set([ | |
| 'authorization', | |
| 'cookie', | |
| 'set-cookie', | |
| 'x-api-key', | |
| 'proxy-authorization', | |
| ]); | |
| const sanitizedHeaders = Object.fromEntries( | |
| Array.from(errorReq.headers.entries()).filter( | |
| ([key]) => !SENSITIVE_HEADERS.has(key.toLowerCase()) | |
| ) | |
| ); | |
| const response = await qstash.publishJSON({ | |
| url, | |
| method, | |
| body: await parseRequestBody(errorReq), | |
| headers: sanitizedHeaders, | |
| retries: 5, | |
| retryDelay, | |
| }); |
Add error handling for QStash publish failures.
If qstash.publishJSON fails (e.g., QStash is down, network issue), the error will propagate and potentially mask the original Prisma error. This could confuse debugging and provide poor user experience.
🔎 Recommended error handling approach
Wrap the QStash publish in a try-catch to handle failures gracefully:
+ try {
const response = await qstash.publishJSON({
url,
method,
body: await parseRequestBody(errorReq),
headers: Object.fromEntries(errorReq.headers.entries()),
retries: 5,
retryDelay,
});
if (response.messageId) {
console.log("Request queued for retry", {
method,
url,
messageId: response.messageId,
});
logger.info("request.retry.queued", {
url,
method,
messageId: response.messageId,
});
await logger.flush();
}
+ } catch (qstashError) {
+ console.error("Failed to queue request for retry", {
+ method,
+ url,
+ error: qstashError,
+ });
+ // Don't throw - we don't want to mask the original error
+ }This ensures that a QStash failure doesn't prevent the original error from being returned to the caller.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/lib/api/queue-failed-request-for-retry.ts around lines 52 to 59, the
call to qstash.publishJSON is not protected so any failure there can mask the
original Prisma error; wrap the publishJSON call in a try-catch, log the publish
error (with context and the payload/URL), do not rethrow so the original error
flow is preserved, and optionally return or record a non-fatal indicator (e.g.,
false) so the caller can continue handling the original error as intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/app/(ee)/api/cron/payouts/process/route.ts (1)
104-110: Make error logging more defensive.Direct access to
error.messageon line 105 could fail if the caught error is not anErrorinstance. Fileapps/web/app/(ee)/api/cron/payouts/process/updates/route.ts(line 172) handles this more defensively.🛡️ Proposed defensive error handling
} catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + await log({ - message: `Error confirming payouts for program: ${error.message}`, + message: `Error confirming payouts for program: ${errorMessage}`, type: "errors", mention: true, }); return handleCronErrorResponse({ error }); }The migration to
handleCronErrorResponse({ error })is correct.Based on learnings, this cron endpoint correctly uses
handleCronErrorResponseto allow QStash to retry errors via its native retry mechanism.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
apps/web/app/(ee)/api/cron/aggregate-clicks/route.tsapps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.tsapps/web/app/(ee)/api/cron/bounties/notify-partners/route.tsapps/web/app/(ee)/api/cron/campaigns/broadcast/route.tsapps/web/app/(ee)/api/cron/cleanup/link-retention/route.tsapps/web/app/(ee)/api/cron/commissions/export/route.tsapps/web/app/(ee)/api/cron/domains/delete/route.tsapps/web/app/(ee)/api/cron/domains/transfer/route.tsapps/web/app/(ee)/api/cron/folders/delete/route.tsapps/web/app/(ee)/api/cron/framer/backfill-leads-batch/route.tsapps/web/app/(ee)/api/cron/fraud/summary/route.tsapps/web/app/(ee)/api/cron/groups/create-default-links/route.tsapps/web/app/(ee)/api/cron/groups/remap-default-links/route.tsapps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.tsapps/web/app/(ee)/api/cron/groups/sync-utm/route.tsapps/web/app/(ee)/api/cron/groups/update-default-links/route.tsapps/web/app/(ee)/api/cron/import/csv/route.tsapps/web/app/(ee)/api/cron/invoices/retry-failed/route.tsapps/web/app/(ee)/api/cron/links/export/route.tsapps/web/app/(ee)/api/cron/links/invalidate-for-discounts/route.tsapps/web/app/(ee)/api/cron/links/invalidate-for-partners/route.tsapps/web/app/(ee)/api/cron/merge-partner-accounts/route.tsapps/web/app/(ee)/api/cron/messages/notify-partner/route.tsapps/web/app/(ee)/api/cron/messages/notify-program/route.tsapps/web/app/(ee)/api/cron/network/calculate-program-similarities/route.tsapps/web/app/(ee)/api/cron/partners/export/route.tsapps/web/app/(ee)/api/cron/payouts/aggregate-due-commissions/route.tsapps/web/app/(ee)/api/cron/payouts/balance-available/route.tsapps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.tsapps/web/app/(ee)/api/cron/payouts/payout-paid/route.tsapps/web/app/(ee)/api/cron/payouts/process/route.tsapps/web/app/(ee)/api/cron/payouts/process/updates/route.tsapps/web/app/(ee)/api/cron/payouts/reminders/program-owners/route.tsapps/web/app/(ee)/api/cron/payouts/send-stripe-payout/route.tsapps/web/app/(ee)/api/cron/send-batch-email/route.tsapps/web/app/(ee)/api/cron/shopify/order-paid/route.tsapps/web/app/(ee)/api/singular/webhook/route.tsapps/web/app/(ee)/api/track/click/route.tsapps/web/app/api/ai/completion/route.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- apps/web/app/(ee)/api/cron/framer/backfill-leads-batch/route.ts
- apps/web/app/(ee)/api/cron/network/calculate-program-similarities/route.ts
- apps/web/app/(ee)/api/cron/fraud/summary/route.ts
- apps/web/app/(ee)/api/cron/messages/notify-partner/route.ts
- apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
- apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts
- apps/web/app/(ee)/api/cron/payouts/aggregate-due-commissions/route.ts
- apps/web/app/(ee)/api/cron/messages/notify-program/route.ts
- apps/web/app/(ee)/api/cron/folders/delete/route.ts
- apps/web/app/(ee)/api/singular/webhook/route.ts
- apps/web/app/(ee)/api/cron/aggregate-clicks/route.ts
- apps/web/app/(ee)/api/cron/import/csv/route.ts
- apps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.ts
- apps/web/app/(ee)/api/cron/bounties/notify-partners/route.ts
- apps/web/app/(ee)/api/cron/groups/update-default-links/route.ts
- apps/web/app/(ee)/api/cron/partners/export/route.ts
- apps/web/app/(ee)/api/cron/payouts/charge-succeeded/route.ts
- apps/web/app/(ee)/api/cron/links/export/route.ts
- apps/web/app/(ee)/api/cron/payouts/send-stripe-payout/route.ts
- apps/web/app/(ee)/api/cron/payouts/reminders/program-owners/route.ts
- apps/web/app/(ee)/api/cron/payouts/payout-paid/route.ts
- apps/web/app/(ee)/api/track/click/route.ts
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:46:01.529Z
Learning: In the Dub codebase, cron endpoints under apps/web/app/(ee)/api/cron/ use handleCronErrorResponse for error handling, which intentionally does NOT detect QStash callbacks or set Upstash-NonRetryable-Error headers. This allows QStash to retry all cron job errors using its native retry mechanism. The selective retry logic (queueFailedRequestForRetry) is only used for specific user-facing API endpoints like /api/track/lead, /api/track/sale, and /api/links to retry only transient Prisma database errors.
Learnt from: devkiran
Repo: dubinc/dub PR: 3207
File: apps/web/lib/cron/with-cron.ts:27-56
Timestamp: 2025-12-09T12:54:41.818Z
Learning: In `apps/web/lib/cron/with-cron.ts`, the `withCron` wrapper extracts the request body once and provides it to handlers via the `rawBody` parameter. Handlers should use this `rawBody` string parameter (e.g., `JSON.parse(rawBody)`) rather than reading from the Request object via `req.json()` or `req.text()`.
📚 Learning: 2025-12-09T12:54:41.818Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3207
File: apps/web/lib/cron/with-cron.ts:27-56
Timestamp: 2025-12-09T12:54:41.818Z
Learning: In `apps/web/lib/cron/with-cron.ts`, the `withCron` wrapper extracts the request body once and provides it to handlers via the `rawBody` parameter. Handlers should use this `rawBody` string parameter (e.g., `JSON.parse(rawBody)`) rather than reading from the Request object via `req.json()` or `req.text()`.
Applied to files:
apps/web/app/(ee)/api/cron/domains/transfer/route.tsapps/web/app/(ee)/api/cron/links/invalidate-for-partners/route.tsapps/web/app/(ee)/api/cron/cleanup/link-retention/route.tsapps/web/app/(ee)/api/cron/send-batch-email/route.tsapps/web/app/(ee)/api/cron/commissions/export/route.tsapps/web/app/(ee)/api/cron/links/invalidate-for-discounts/route.tsapps/web/app/(ee)/api/cron/groups/sync-utm/route.tsapps/web/app/(ee)/api/cron/payouts/process/updates/route.tsapps/web/app/(ee)/api/cron/domains/delete/route.tsapps/web/app/(ee)/api/cron/payouts/process/route.tsapps/web/app/(ee)/api/cron/invoices/retry-failed/route.tsapps/web/app/(ee)/api/cron/merge-partner-accounts/route.tsapps/web/app/(ee)/api/cron/payouts/balance-available/route.ts
📚 Learning: 2025-12-15T16:45:51.667Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3213
File: apps/web/app/(ee)/api/cron/auto-approve-partner/route.ts:122-122
Timestamp: 2025-12-15T16:45:51.667Z
Learning: In cron endpoints under apps/web/app/(ee)/api/cron, continue using handleCronErrorResponse for error handling. Do not detect QStash callbacks or set Upstash-NonRetryable-Error headers in these cron routes, so QStash can retry cron errors via its native retry mechanism. The existing queueFailedRequestForRetry logic should remain limited to specific user-facing API endpoints (e.g., /api/track/lead, /api/track/sale, /api/links) to retry only transient Prisma/database errors. This pattern should apply to all cron endpoints under the cron directory in this codebase.
Applied to files:
apps/web/app/(ee)/api/cron/domains/transfer/route.tsapps/web/app/(ee)/api/cron/shopify/order-paid/route.tsapps/web/app/(ee)/api/cron/links/invalidate-for-partners/route.tsapps/web/app/(ee)/api/cron/cleanup/link-retention/route.tsapps/web/app/(ee)/api/cron/send-batch-email/route.tsapps/web/app/(ee)/api/cron/commissions/export/route.tsapps/web/app/(ee)/api/cron/links/invalidate-for-discounts/route.tsapps/web/app/(ee)/api/cron/groups/sync-utm/route.tsapps/web/app/(ee)/api/cron/payouts/process/updates/route.tsapps/web/app/(ee)/api/cron/groups/create-default-links/route.tsapps/web/app/(ee)/api/cron/domains/delete/route.tsapps/web/app/(ee)/api/cron/payouts/process/route.tsapps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.tsapps/web/app/(ee)/api/cron/invoices/retry-failed/route.tsapps/web/app/(ee)/api/cron/merge-partner-accounts/route.tsapps/web/app/(ee)/api/cron/payouts/balance-available/route.ts
📚 Learning: 2025-06-06T07:59:03.120Z
Learnt from: devkiran
Repo: dubinc/dub PR: 2177
File: apps/web/lib/api/links/bulk-create-links.ts:66-84
Timestamp: 2025-06-06T07:59:03.120Z
Learning: In apps/web/lib/api/links/bulk-create-links.ts, the team accepts the risk of potential undefined results from links.find() operations when building invalidLinks arrays, because existing links are fetched from the database based on the input links, so matches are expected to always exist.
Applied to files:
apps/web/app/(ee)/api/cron/links/invalidate-for-partners/route.tsapps/web/app/(ee)/api/cron/cleanup/link-retention/route.tsapps/web/app/(ee)/api/cron/groups/create-default-links/route.tsapps/web/app/(ee)/api/cron/merge-partner-accounts/route.ts
📚 Learning: 2025-11-17T05:19:11.972Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3113
File: apps/web/app/(ee)/api/cron/payouts/charge-succeeded/send-paypal-payouts.ts:65-75
Timestamp: 2025-11-17T05:19:11.972Z
Learning: In the Dub codebase, `sendBatchEmail` (implemented in packages/email/src/send-via-resend.ts) handles filtering of emails with invalid `to` addresses internally. Call sites can safely use non-null assertions on email addresses because the email sending layer will filter out any entries with null/undefined `to` values before sending. This centralized validation pattern is intentional and removes the need for filtering at individual call sites.
Applied to files:
apps/web/app/(ee)/api/cron/send-batch-email/route.ts
📚 Learning: 2025-12-03T09:19:48.164Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3175
File: apps/web/lib/actions/partners/bulk-reject-partner-applications.ts:14-21
Timestamp: 2025-12-03T09:19:48.164Z
Learning: In apps/web/lib/actions/partners/bulk-reject-partner-applications.ts, the bulkRejectPartnerApplicationsAction does not need explicit plan capability checks for fraud operations (when reportFraud is true) because the authorization is handled automatically by the underlying fraud operation functions (resolveFraudGroups, createFraudEvents) or through other automated mechanisms in the system.
Applied to files:
apps/web/app/(ee)/api/cron/merge-partner-accounts/route.ts
📚 Learning: 2025-11-24T09:10:12.536Z
Learnt from: devkiran
Repo: dubinc/dub PR: 3089
File: apps/web/lib/api/fraud/fraud-rules-registry.ts:17-25
Timestamp: 2025-11-24T09:10:12.536Z
Learning: In apps/web/lib/api/fraud/fraud-rules-registry.ts, the fraud rules `partnerCrossProgramBan` and `partnerDuplicatePayoutMethod` intentionally have stub implementations that return `{ triggered: false }` because they are partner-scoped rules handled separately during partner application/onboarding flows (e.g., in detect-record-fraud-application.ts), rather than being evaluated per conversion event like other rules in the registry. The stubs exist only to satisfy the `Record<FraudRuleType, ...>` type constraint.
Applied to files:
apps/web/app/(ee)/api/cron/merge-partner-accounts/route.ts
🧬 Code graph analysis (16)
apps/web/app/(ee)/api/cron/domains/transfer/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/shopify/order-paid/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/links/invalidate-for-partners/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/cleanup/link-retention/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/send-batch-email/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/commissions/export/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/links/invalidate-for-discounts/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/groups/sync-utm/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/payouts/process/updates/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/groups/create-default-links/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/domains/delete/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/invoices/retry-failed/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/merge-partner-accounts/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/(ee)/api/cron/payouts/balance-available/route.ts (1)
apps/web/app/(ee)/api/cron/utils.ts (1)
handleCronErrorResponse(22-33)
apps/web/app/api/ai/completion/route.ts (1)
apps/web/lib/api/errors.ts (1)
handleAndReturnErrorResponse(168-197)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (28)
apps/web/app/(ee)/api/cron/cleanup/link-retention/route.ts (1)
9-9: LGTM! Cron error handling correctly updated.The changes align with the systematic refactoring to use
handleCronErrorResponsefor cron endpoints. This properly allows QStash to handle retries through its native retry mechanism, as indicated in the learnings.Based on learnings, this pattern is correct for cron endpoints and differs from user-facing API endpoints that use selective retry logic.
Also applies to: 52-52
apps/web/app/(ee)/api/cron/groups/create-default-links/route.ts (1)
15-15: LGTM! Error handling refactoring aligns with cron-specific pattern.The changes correctly update the error handling to use
handleCronErrorResponse, which is the established pattern for cron endpoints in this codebase. This allows QStash to handle retries via its native retry mechanism for all cron job errors, which is the intended behavior.Based on learnings, this pattern is consistently applied across cron endpoints, while the selective retry logic for transient Prisma errors is reserved for specific user-facing API endpoints.
Also applies to: 191-191
apps/web/app/(ee)/api/cron/groups/sync-utm/route.ts (2)
12-12: LGTM! Import change aligns with cron error handling pattern.The switch to
handleCronErrorResponseis correct for cron endpoints. Based on learnings, this handler intentionally omits QStash callback detection and retry headers, allowing QStash's native retry mechanism to handle all cron job errors.
150-150: LGTM! Error handler call correctly updated.The call signature properly uses the object parameter
{ error }, matching thehandleCronErrorResponsefunction signature. The error logging flow is preserved, and this change aligns with the broader refactoring to standardize cron endpoint error handling.apps/web/app/(ee)/api/cron/commissions/export/route.ts (1)
13-13: LGTM! Error handling refactored correctly for cron endpoints.The import and usage of
handleCronErrorResponsealign with the codebase's error handling strategy for cron workers. The signature change fromhandleAndReturnErrorResponse(error)tohandleCronErrorResponse({ error })is correct, and error logging is preserved.Based on learnings, this approach correctly allows QStash to retry cron job errors via its native retry mechanism, rather than using selective retry logic.
Also applies to: 111-111
apps/web/app/(ee)/api/cron/shopify/order-paid/route.ts (2)
1-6: LGTM! Clean migration to cron-specific error handler.The import changes are correct:
DubApiErroris retained (still used on line 63).handleCronErrorResponseis imported from the correct relative path.This aligns with the established pattern for cron endpoints. Based on learnings,
handleCronErrorResponseintentionally does NOT set Upstash-NonRetryable-Error headers, allowing QStash to retry cron job errors via its native retry mechanism.
69-71: Error handling correctly migrated.The catch block now uses
handleCronErrorResponse({ error }), which is the correct pattern for cron endpoints. This ensures QStash can handle retries naturally without interference from custom retry logic.apps/web/app/(ee)/api/cron/payouts/balance-available/route.ts (2)
18-18: LGTM! Import correctly updated for cron-specific error handling.The import change from
handleAndReturnErrorResponsetohandleCronErrorResponsealigns with the established pattern for cron endpoints. This allows QStash to handle retries natively without custom retry logic for transient errors.Based on learnings, this pattern is correctly applied across all cron endpoints in this codebase.
205-212: LGTM! Error handling correctly migrated to cron-specific handler.The catch block correctly uses
handleCronErrorResponse({ error })with the object parameter syntax. Error logging is preserved before the handler call, and the change aligns with the cron endpoint pattern where QStash handles retries natively.Based on learnings, this pattern correctly avoids setting Upstash-NonRetryable-Error headers, allowing QStash to retry all cron job errors using its native retry mechanism.
apps/web/app/(ee)/api/cron/send-batch-email/route.ts (2)
9-9: LGTM! Correct migration to cron-specific error handler.The import change from
handleAndReturnErrorResponsetohandleCronErrorResponsealigns with the standardized error handling pattern for cron endpoints. This allows QStash to manage retries through its native retry mechanism.Based on learnings, this pattern is appropriate for all cron endpoints under
/api/cron.
194-194: LGTM! Correct usage of the new error handler signature.The error handler call correctly uses the object parameter format
{ error }, which matches the signature ofhandleCronErrorResponseas defined inapps/web/app/(ee)/api/cron/utils.ts. The error logging on lines 188-192 is preserved appropriately.apps/web/app/(ee)/api/cron/domains/delete/route.ts (2)
12-12: LGTM! Import aligns with cron-specific error handling pattern.The import of
handleCronErrorResponseis correct and consistent with the PR's systematic refactoring of error handling across cron routes.
137-137: LGTM! Error handling properly uses cron-specific handler.The change to
handleCronErrorResponse({ error })is correct and aligns with the design decision to allow QStash's native retry mechanism for all cron job errors.Based on learnings, this pattern ensures QStash can retry failures without the Upstash-NonRetryable-Error header being set.
apps/web/app/(ee)/api/cron/domains/transfer/route.ts (2)
9-9: LGTM! Import follows the established pattern.The import of
handleCronErrorResponseis consistent with the systematic refactoring across all cron routes in this PR.
127-127: LGTM! Error handling correctly implemented for cron endpoint.The change to
handleCronErrorResponse({ error })is appropriate and maintains the existing logging while aligning with QStash's native retry mechanism.Based on learnings, this ensures all cron job errors can be retried by QStash without Upstash-NonRetryable-Error headers.
apps/web/app/(ee)/api/cron/links/invalidate-for-partners/route.ts (2)
5-5: LGTM! Correct import for cron-specific error handling.The switch to
handleCronErrorResponsefrom the local cron utilities aligns with the PR's systematic refactoring pattern for cron endpoints. This allows QStash to handle retries natively without selective retry logic.Based on learnings, this pattern is intentional for all cron endpoints.
52-52: LGTM! Correct error handler invocation.The updated call signature
handleCronErrorResponse({ error })correctly matches the new object parameter pattern, maintaining proper error handling for this cron endpoint.apps/web/app/(ee)/api/cron/links/invalidate-for-discounts/route.ts (2)
6-6: LGTM! Correct import for cron-specific error handling.The switch to
handleCronErrorResponsefrom the local cron utilities aligns with the PR's systematic refactoring pattern. The import also correctly includeslogAndRespondwhich is used throughout the file (lines 37, 63, 71, 84).Based on learnings, this pattern is intentional for all cron endpoints.
86-86: LGTM! Correct error handler invocation.The updated call signature
handleCronErrorResponse({ error })correctly matches the new object parameter pattern, maintaining proper error handling for this cron endpoint.apps/web/app/(ee)/api/cron/merge-partner-accounts/route.ts (2)
15-15: LGTM! Error handler import standardized.The import change to
handleCronErrorResponsecorrectly standardizes error handling for this cron endpoint, consistent with the broader refactoring across cron routes.
417-417: LGTM! Error response handling correctly updated.The error handler call is correctly updated to use the new object parameter signature. Based on learnings, this maintains the intended behavior where QStash can retry cron errors via its native retry mechanism without selective retry logic.
apps/web/app/(ee)/api/cron/payouts/process/route.ts (1)
6-6: LGTM! Correct migration to cron-specific error handler.The import change correctly brings in
handleCronErrorResponseandlogAndRespondfor standardized cron error handling.Based on learnings, cron endpoints should use
handleCronErrorResponseto allow QStash native retry mechanism.apps/web/app/(ee)/api/cron/payouts/process/updates/route.ts (2)
9-9: LGTM! Correct migration to cron-specific error handler.The import correctly brings in
handleCronErrorResponseandlogAndRespondfrom the cron utils module.Based on learnings, cron endpoints should use
handleCronErrorResponseto allow QStash native retry mechanism.
171-180: LGTM! Excellent defensive error handling.The error handling correctly:
- Uses defensive type checking on line 172 (
error instanceof Error)- Logs errors with mention flag for visibility
- Calls
handleCronErrorResponse({ error })with the proper object parameterBased on learnings, this cron endpoint correctly uses
handleCronErrorResponseto allow QStash to retry errors via its native retry mechanism.apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts (1)
6-6: LGTM! Correctly aligned with cron error handling pattern.The changes properly migrate this cron endpoint to use
handleCronErrorResponsewith the new object parameter signature{ error }. This aligns with the established pattern for cron routes, allowing QStash to handle retries through its native retry mechanism rather than using selective retry logic.Based on learnings, this is the correct approach for cron endpoints.
Also applies to: 110-110
apps/web/app/api/ai/completion/route.ts (1)
57-57: Verify ifrequestHeadersshould be passed to enable complete retry detection.The error handler call correctly matches the new signature by wrapping the error in an object. However, the updated
handleAndReturnErrorResponsefunction (shown in the relevant snippets) now includes QStash callback detection logic that checks forrequestHeaders?.get("upstash-signature").Since
requestHeadersis not passed here, the QStash retry detection and theUpstash-NonRetryable-Errorheader logic won't apply to this endpoint. Thereq.headersobject is available in scope and could be passed asrequestHeaders: req.headers.Is this intentional because
/api/ai/completionis not a cron endpoint and won't receive QStash callbacks, or should headers be passed for consistency with the PR's retry mechanism?apps/web/app/(ee)/api/cron/invoices/retry-failed/route.ts (2)
5-5: LGTM! Correct migration to cron-specific error handler.The import correctly references the handleCronErrorResponse function from the local cron utils module, aligning with the systematic refactoring across cron endpoints.
Based on learnings, this allows QStash to retry cron job errors via its native retry mechanism.
82-82: LGTM! Error handling updated correctly.The catch block correctly calls handleCronErrorResponse with the object parameter format
{ error }, matching the function signature defined in the cron utils module.Based on learnings, this standardized error handling approach is appropriate for cron endpoints.
Summary by CodeRabbit
Bug Fixes
Infrastructure
✏️ Tip: You can customize this high-level summary in your review settings.