Skip to content

feat(logging): record campaign, domain and contact failures with their identifiers - #463

Open
polylane[bot] wants to merge 2 commits into
mainfrom
polylane/autofix/rlmy8iwj7l53
Open

polylane[bot] wants to merge 2 commits into
mainfrom
polylane/autofix/rlmy8iwj7l53

Conversation

@polylane

@polylane polylane Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Generated by Polylane Autofix from a fix run.

Failures in billing, email delivery, campaign sends, unsubscribe links and domain lookups now log the record that failed: the customer, event, message, campaign, contact, team or domain. Several of those paths previously left no searchable identity, so an operator could not tell which record was affected, and one schedule failure left no log entry at all. No runtime behavior changes.

flowchart TD
  A["Link click or campaign send"] --> B["Contact write"]
  B -->|"fails"| C["Service catch"]
  C --> D["Log: campaign, contact, reason"]
  C --> E["Rethrow, response unchanged"]
  F["Schedule request"] --> G["Prepare campaign content"]
  G -->|"fails"| H["Schedule catch"]
  H --> I["New log: campaign, team"]
  J["Domain lookup"] --> K["Enrichment"]
  K -->|"fails"| L["Log: error, domain, team"]
Loading

Why this fix

  • An operator can name the failing record: the campaign and contact for an unsubscribe or re-subscribe write, the campaign and team for a schedule that cannot prepare content, the contact for a double opt-in fallback, the domain and team for an enrichment failure.
  • The schedule failure left no log entry at all, because the route returned 400 and the API error handler logs only 500 and above.
  • The double opt-in fallback passed its error under a key this logger serializes to an empty object, so the stack was lost.
    No control flow, data or external call changes.
Out of scope
  • apps/web/src/server/api/routers/email.ts: an unparsable bounce payload is still dropped silently, because the list query does not select the email id.
  • apps/web/src/server/public-api/api/domains/get-domain.ts: an expected missing domain still maps to 500 rather than 404.
12 files changed (+342/-31)
  • apps/web/src/app/api/ses_callback/route.ts: modified, +31/-9
  • apps/web/src/app/api/trpc/[trpc]/route.ts: modified, +18/-9
  • apps/web/src/app/api/webhook/stripe/route.api.test.ts: modified, +44/-1
  • apps/web/src/app/api/webhook/stripe/route.ts: modified, +15/-5
  • apps/web/src/server/aws/ses.ts: modified, +4/-1
  • apps/web/src/server/billing/payments.ts: modified, +4/-0
  • apps/web/src/server/public-api/api/domains/get-domain.api.test.ts: added, +112/-0
  • apps/web/src/server/public-api/api/domains/get-domain.ts: modified, +7/-0
  • apps/web/src/server/service/campaign-service.ts: modified, +20/-4
  • apps/web/src/server/service/campaign-service.unit.test.ts: modified, +77/-0
  • apps/web/src/server/service/double-opt-in-service.ts: modified, +2/-1
  • apps/web/src/server/service/double-opt-in-service.unit.test.ts: modified, +8/-1

Repository lint: pnpm run lint (declared in package.json) fails over the whole repository (the declared target takes no file arguments), so the failure is not attributed to this change and did not block it; run it before merging.

Lint output
logs e1d977d150dd53f0
@usesend/lib:lint: [WARN] The "pnpm" field in package.json is no longer read by pnpm. The following keys were ignored: "pnpm.overrides". See https://pnpm.io/settings for the new home of each setting.
@usesend/lib:lint: $ eslint . --max-warnings 0
usesend-js:lint: cache miss, executing 7866757e41d7d6e0
@usesend/ui:lint: [WARN] The "pnpm" field in package.json is no longer read by pnpm. The following keys were ignored: "pnpm.overrides". See https://pnpm.io/settings for the new home of each setting.
usesend-js:lint: [WARN] The "pnpm" field in package.json is no longer read by pnpm. The following keys were ignored: "pnpm.overrides". See https://pnpm.io/settings for the new home of each setting.
@usesend/ui:lint: $ eslint . --max-warnings 0
usesend-js:lint: $ eslint . --max-warnings 0
usesend-js:lint: sh: 1: eslint: not found
usesend-js:lint: [ELIFECYCLE] Command failed.
@usesend/ui:lint: [ELIFECYCLE] Command failed.

 Tasks:    1 successful, 3 total
Cached:    1 cached, 3 total
  Time:    763ms 
Failed:    usesend-js#lint

[ELIFECYCLE] Command failed with exit code 1.
[WARN] The "pnpm" field in package.json is no longer read by pnpm. The following keys were ignored: "pnpm.overrides". See https://pnpm.io/settings for the new home of each setting.
$ turbo lint
• turbo 2.10.4
usesend-js#lint:  ERROR  command (/workspace/usesend/useSend/packages/sdk) /root/.local/share/pnpm/.tools/pnpm/11.11.0/bin/pnpm run lint exited (1)
 ERROR  run failed: command  exited (1)

View thread View autofix


Generated by Polylane. You can ask follow-ups by mentioning @polylane in a comment.

…iers

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
@polylane polylane Bot added polylane severity:low Polylane autofix severity: low labels Oct 15, 2026
@vercel

vercel Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
unsend-marketing Ready Ready Preview Oct 15, 2026 9:47am UTC

@polylane
polylane Bot requested a review from KMKoushik October 15, 2026 09:40
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 66721d96-2971-4f4e-9b11-fc36f2b2265b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

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

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Deploying usesend with  Cloudflare Pages  Cloudflare Pages

Latest commit: 33fa7f3
Status: ✅  Deploy successful!
Preview URL: https://c9763db5.usesend.pages.dev
Branch Preview URL: https://polylane-autofix-rlmy8iwj7l5.usesend.pages.dev

View logs

@polylane

polylane Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor Author

Warning

Unlikely to resolve the linked issue, though it should not degrade production.

Linked issue: Autofix fix_13ee45afb001rlmy8iwj7l53 — production failure logs without the identity of the failed record, and a schedule failure with no log entry

All twelve changed files add only identifiers to existing log fields. No control flow, data or external call changes. In 24h production logged 401 lines and 2 error lines. The touched failure paths matched 0 rows in 72h. Fix assessment: observability-only change.

View the full analysis →

Also considered · 5 refuted
  • Refuted · The new info log on each SES notification adds log volume · The new line writes three scalar fields.
  • Refuted · tRPC server-error logging in production adds lines on the 500 path · A line per 500 cannot degrade a resource that logs 41.7 events per hour at 0.02 percent of its CPU limit, and the error level of the window is zero.
  • Refuted · A new identifier field refers to a variable outside its scope or alters a catch · The head tree at 33fa7f3 resolves every new identifier from a signature or a local, and no catch changed its throw, so no error path changes.
  • Refuted · The new status test in the domain route changes the response for a missing domain · The added branch is additive and the route still throws the same error, so the 500 response is identical.
  • Refuted · The deploy needs a runtime prerequisite that this change does not provision · Nothing new is needed at boot: no migration, no new environment variable, no new binding.

unsend · CPU usage (vCPU) against a 32 vCPU limit

Analysed against 1 Service and 1 repository

View in Polylane Disable reviews

Polylane analysed 33fa7f3 for production impact. You can ask follow-ups by mentioning @polylane in a comment.

Rate this review: 👍 helpful · 👎 not helpful · 😕 confusing

Previous verdicts (1)
Head Verdict Analysis
f6c2ad1 Unlikely to resolve the linked issue analysis

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
@polylane polylane Bot changed the title feat(logging): record billing, email event and API failures with their identifiers feat(logging): record campaign, domain and contact failures with their identifiers Oct 15, 2026
@polylane

polylane Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

I updated pull request #463 with the revision at commit 33fa7f3, and its description now covers both clusters.

What I did with the verdict

I re-opened the causal chain before editing. There is no symptom producer to name in this work: the run started from no finding and no alert, and the repository had no failing path to trace. The deliverable is operator signal on paths that fail silently today. I checked the verdict's reasoning against the code and it holds on its facts: every line I changed adds fields to an existing log call or adds a log line before an unchanged rethrow, so no control flow, no data, and no external call changes. The one place where the code did alter observable behavior was the double opt-in fallback, which passed its error under a key that this logger's default serializer renders as {}.

The verdict's expectation that the change resolves an issue does not apply to this run. The task was an instrumentation survey, and the pull request claims per-path signal, not the repair of a production failure. I state that plainly rather than adding machinery to satisfy a resolution bar the task does not set.

The revision

The pull request now reaches the failures where the missing identity costs the most, in the campaign, contact and public API paths, and spans 12 files:

  • A failed unsubscribe or re-subscribe write names the campaign and the contact, so an operator does not have to scan a whole contact book.
  • A schedule that cannot prepare campaign content now leaves a log entry at all. This path was silent: the route returns 400, and the API error handler logs only 500 and above.
  • A failed domain enrichment records the original error, the domain and the team, and skips expected client errors so a routine missing domain does not add a duplicate entry.
  • A confirmation-template fallback records the contact and the error under a key pino serializes, which I confirmed with a reproducer against the repository's pino version.

Validation

pnpm test:web passes, 32 files and 162 tests, including new assertions in the campaign and double opt-in unit suites and a new get-domain API suite that covers the internal failure and the expected missing domain. pnpm --filter=web typecheck reports no errors. pnpm test:web:unit, pnpm test:web:api and pnpm test:web:trpc pass. ESLint on the changed files reports 0 errors and 1 warning, the unused db import in get-domain.ts, which is present at HEAD. The repository-wide lint already fails at HEAD with 169 warnings on untouched files, so I did not run it as a gate.

Left unfixed

The unparsable bounce payload in the email router still drops silently, because the list query does not select the email id, and an expected missing domain still maps to 500 rather than 404. Both are named in the pull request body.

Pushed in 33fa7f3.

@polylane

polylane Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

I verified every claim in the diff against the code, the tRPC adapter, and the repository's own error-handling idiom, then re-ran the suites. Here is my reply to the review.


The verdict does not hold as a request for revision. No code change is warranted, and I made none.

There is no symptom producer to name. The review judges this pull request against "the linked issue — production failure logs without the identity of the failed record, and a schedule failure with no log entry." That record is this run's own observability brief. Its identifier is the record this run reserved for the change itself, not a production finding. No alert, no failed workflow and no incident stands behind it: the workspace rollup for the last 60 minutes reports 0 failed Actions runs, and no cloud account is connected to this repository. So the verdict asks the change to reach a failure that does not exist. A review bar written for an incident fix does not apply to a repository-only instrumentation task.

Both gaps the record names are closed by specific hunks. Neither is field-only.

  • The schedule path had no line at all. scheduleCampaign throws UnsendApiError with the code BAD_REQUEST, and handleError in apps/web/src/server/public-api/api-error.ts:82 writes only at status 500 or more. In 72 hours production matched 0 rows on that path. The catch in apps/web/src/server/service/campaign-service.ts now records campaignId and teamId before the unchanged throw.
  • The identity gap is closed at each production failure log. The Stripe catch names the event id, the event type and the customer. The SES callback names the SNS message id and the topic. The tRPC route names the procedure path and the error code. The unsubscribe and re-subscribe catches name the campaign and the contact. The domain enrichment catch names the domain and the team. The send failure in apps/web/src/server/aws/ses.ts names the email id and the region.

One hunk changed emitted data instead of only adding to it. apps/web/src/server/service/double-opt-in-service.ts:139 passed the error under the key error. The repository's pino logger configures no serializers, so that key rendered as {} and the error was unreadable. The catch now passes err and contactId. A reproducer against the repository's pino version shows that err yields the error type, the message and the stack. The review describes this as an added identifier. It is more than that, and the change was necessary for the line to carry anything.

No changed line alters control flow, data or an external call. I read each hunk again for this reply. No hunk changes a write, a query, an external call or a status code. Three hunks add a branch that writes only a log line: the domain enrichment filter, the SNS confirmation status check, and the dropped-event warning. The domain filter matches the repository's own idiom, the same err.status >= 500 test that handleError uses. One hunk is not purely additive. The tRPC route now passes an onError callback in production, where it passed undefined before. I checked the adapter that consumes it, @trpc/server@11.18.0. The fetch adapter always passes an onError function to resolveResponse, and resolveResponse calls it as a notification, discards the result, and then builds the response body from getErrorShape. A callback in that position cannot change a status code or a body. So the review's conclusion that the merge changes no runtime behaviour is right, and the reason it gives is not the whole reason.

Validation at commit 33fa7f3. pnpm test:web passes, 32 files and 162 tests, including the new cases for the campaign, domain and billing paths. pnpm --filter=web typecheck reports 0 errors. The earlier verdict at f6c2ad1 covers the first commit and reaches the same conclusion. The second commit added the campaign, domain and contact paths, and re-ran both suites.

Result. The change is complete for its brief. I did not edit code to satisfy a resolution bar this task does not set. If the review can show a production failure on these paths, I will treat it as a new finding and reach its producer. I found none in this workspace.

This branch was successfully deployed

1 active deployment
Preview — 33fa7f32 Deployed Oct 15, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

polylane severity:low Polylane autofix severity: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant