Skip to content

fix(messaging): refuse a transactional email to a recipient that can't receive email - #123878

Merged
trunk-io[bot] merged 2 commits into
masterfrom
posthog/transactional-email-refuse-unreachable-recipient
Oct 27, 2026
Merged

trunk-io[bot] merged 2 commits into
masterfrom
posthog/transactional-email-refuse-unreachable-recipient

Conversation

@mayteio

@mayteio mayteio commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Problem

  • A transactional email API caller who sends to an invalid address, or to a domain with no mail servers, gets 202 queued, but the email never goes out.
  • The email worker skips that send because it would hard bounce, so the caller believes an email went out that never will.
  • The cross-SDK contract in Transactional email SDK contract for posthog-python and posthog-node #120921 says every check that would skip a send runs before the 202. The recipient check did not.

Refs #120921

Changes

  • A send to an unreachable recipient now gets 422 with the new error code recipient_unreachable and a message that asks the caller to check the address for a typo.
  • refuseTransactionalEmail in the Node CDP API runs the same recipient check as the worker, through the new EmailValidationService.getRecipientSkipReason(teamId, email).
  • getRecipientSkipReason is split out of getSkipReason, so the worker and the API share one check.
  • The check runs last among the refusals, because a cache miss costs a DNS lookup.
  • DNS failures fail open, so a DNS outage never blocks email.
  • The worker keeps its own check, because state can change while the job waits in the queue.
  • Django maps the new pipeline code: RecipientUnreachable contract, a PIPELINE_REFUSALS entry, and a 422 in TransactionalEmailRejected.STATUS_BY_CODE.
  • Mechanical: createCdpCoreServices exposes emailValidationService, and the 422 OpenAPI description mentions the new case.

How did you test this code?

Test rationale: The new case in cdp-api.serial.test.ts ("refuses a send the email worker would skip because the recipient's domain has no mail servers") catches the API answering 202 for a send the worker would skip. It extends the existing parameterized refusal test. With the new refuseTransactionalEmail lines removed, it fails (202 instead of 422). The new row in test_reports_a_send_the_pipeline_refuses catches Django returning a server error for the new pipeline code. A beforeEach stub makes getRecipientSkipReason return null for the other cases, because the fixtures send to example.com, which publishes a null MX.

Run locally:

  • cdp-api.serial.test.ts with -t "transactional email", and email-validation.service.test.ts.
  • products/messaging/backend/tests/api/test_transactional_email.py.
  • Repo-wide mypy, ruff, eslint and prettier on the changed files.

Not run: a real send against a live DNS resolver, or manual testing in a running stack.

Release status

  • This change is behind a feature flag and is not available to users

Docs update

None in this repo. The transactional email docs page (PostHog/posthog.com#21564) should list the new recipient_unreachable error code.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: PostHog Desktop (Claude Code), Claude Opus 5.5

  • Skills invoked: /writing-pr-descriptions.
  • Duplicate search: gh pr list --state open --search "recipient unreachable transactional" found no open PR.
  • Public artifact: all test data is invented and uses example.com. Nothing in this PR comes from customer material.

🤖 Generated with Claude Code


Created with PostHog Desktop

…t receive email

The transactional email API answered 202 for a send the email worker
later skipped because the recipient address is invalid or its domain
has no mail servers. The API now runs the same recipient check before
it answers and refuses with 422 recipient_unreachable.

EmailValidationService.getRecipientSkipReason splits the per-address
check out of getSkipReason, so the worker and the API share it. The API
runs it last, because a cache miss costs a DNS lookup. DNS failures
fail open. Django maps the new pipeline code to RecipientUnreachable.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

Generated-By: PostHog Desktop
Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
@mayteio mayteio self-assigned this Oct 27, 2026
@trunk-io

trunk-io Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane, mixes lanes

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

This PR changes Python or frontend code (products/messaging/backend/facade/contracts.py, products/messaging/backend/presentation/views/message_templates.py, products/messaging/backend/services/transactional_email.py and 1 more) and Node or Rust code (nodejs/src/cdp/cdp-api.serial.test.ts, nodejs/src/cdp/cdp-api.ts, nodejs/src/cdp/cdp-services.ts and 2 more) together. In the merge queue, every Node or Rust PR behind it in the same lane then runs the Django and frontend suites too, so the cross-lane check fails. Split it, or add the cross-lane-change label if the halves must land together.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

@trunk-io

trunk-io Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
EmailService native email sending with maildev should include tracking code in the email with distinct_id The test expected one matching item but received an empty array. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

mayteio commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review --deep

mayteio commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@mayteio mayteio added the posthog-review Run a Standard PostHog Review label Oct 27, 2026 — with PostHog

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 5289444: no findings.

This Standard review started from the posthog-review label. Comment @posthog review --deep for a Deep review, or @posthog help to see all commands.

@posthog posthog Bot removed the posthog-review Run a Standard PostHog Review label Oct 27, 2026
@mayteio
mayteio marked this pull request as ready for review October 27, 2026 10:43
@mayteio mayteio added the stamphog Request AI approval (no full review) label Oct 27, 2026 — with PostHog
@parameterai

parameterai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Risk: No findings

Since the last review this PR only merged master and updated a test to match master's new behavior (quota-limited transactional sends queue instead of refusing), plus added two echo fields to the preferences-token validation response. The two new response fields are claims from a server-signed, fail-closed-verified JWT, so they disclose nothing the token holder could not already decode from the token itself, and the recipient-unreachable refusal chain is intact after the merge.

Sentinel reviewed 95ea844 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 27, 2026 10:44

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

The disclosed API and send-path changes have independent assurance from a current-head review and owning-team authorship. No unresolved substantive review concerns or other policy refusal grounds were found.

  • Author wrote 50% of the modified lines and has 39 merged PRs in these paths (familiarity MODERATE).
  • PostHog Review (Standard) reviewed the current head: no findings.
  • posthog[bot] reviewed the current head.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 41L, 7F substantive, 52L/9F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (52L, 9F, two-areas, fix)
stamphog 2.6.1 .stamphog/policy.yml @ 5289444 · reviewed head 5289444

@mayteio mayteio added the cross-lane-change Allow for cross lane (i.e. django + rust) changes to merge at once label Oct 27, 2026 — with PostHog
…able-recipient

Generated-By: PostHog Desktop
Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
@trunk-io
trunk-io Bot merged commit 375a260 into master Oct 27, 2026
260 checks passed
@trunk-io
trunk-io Bot deleted the posthog/transactional-email-refuse-unreachable-recipient branch October 27, 2026 16:43

mayteio commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@trunk-io

trunk-io Bot commented Oct 27, 2026

Copy link
Copy Markdown

An error occurred while submitting your PR to the queue: This PR is already merged

@deployment-status-posthog

deployment-status-posthog Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-27 17:36 UTC Run
prod-us ✅ Deployed 2026-10-27 18:00 UTC Run
prod-eu ✅ Deployed 2026-10-27 17:55 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cross-lane-change Allow for cross lane (i.e. django + rust) changes to merge at once stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant