Repository navigation
Conversation
Add posthog.emails.send() for PostHog transactional email templates, with the appHost, emailRequestTimeoutMs and emailRequestMaxRetries client options. Sends go to the app host with the project secret key and an idempotency key that every retry reuses. send() resolves to { data, error }.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-By: PostHog Desktop
Task-Id: a0274fa0-d9be-48bf-b601-f153decf241d
Coherence0 spec problems · 0 failing chokepoints · 2 spec gaps · 0 guard failures 44 components, 7 bullets (5 invariants, 2 requirements), 0 problems Chokepoints: none declared. Spec gaps: 2 entrances with outside or unknown trust and no traced control, and 10 more held by the adoption baseline
Close one with |
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 65d7240: 3 Should fix.
This Standard review started from the posthog-review label. Comment @posthog review --deep for a Deep review, or @posthog help to see all commands.
See this review and its findings in PostHog Review.
|
|
||
| function readRetryAfter(response: PostHogFetchResponse): number | undefined { | ||
| try { | ||
| return parseRetryAfterMs(response.headers?.get('retry-after')) |
There was a problem hiding this comment.
P2 · Preserve the full server Retry-After duration
When PostHog returns Retry-After: 3600, readRetryAfter returns 300000 ms because the shared parseRetryAfterMs caps values at five minutes. Consequently, error.retryAfterMs tells callers to retry 55 minutes early, breaking server-directed backoff in job runners. Parse the full duration for email errors and apply the retry ceiling separately.
There was a problem hiding this comment.
Fixed in cc3b6f6. readRetryAfter now passes a cap to parseRetryAfterMs (a new optional argument in @posthog/core), so error.retryAfterMs keeps the full server wait. The 30 second retry ceiling in retryDelayMs still applies separately. The existing "gives up at once on a Retry-After over 30 seconds" test now uses Retry-After: 3600 and expects 3600000 ms, which fails with the five minute cap.
| let timer: ReturnType<typeof safeSetTimeout> | ||
| // `fetch` is injectable and may ignore the abort signal, so both phases race the deadline. | ||
| const deadline = new Promise<never>((_resolve, reject) => { | ||
| timer = safeSetTimeout(() => { |
There was a problem hiding this comment.
P2 · Keep the request deadline referenced until the send settles
When an injected fetch stalls without a referenced I/O handle, safeSetTimeout unrefs the only timer keeping request() alive. A short-lived Node script exits successfully while awaiting emails.send(), before the timeout fires or retries run, so callers never receive an error. Keep this deadline referenced with setTimeout, as the retry sleep already does.
There was a problem hiding this comment.
Fixed in cc3b6f6. The deadline uses a plain setTimeout instead of safeSetTimeout, so it stays referenced until the send settles, the same as the retry sleep. finally still clears it. No new test: an unref'd timer only changes whether a real process exits, and the suite runs on fake timers.
| if (!appHost) { | ||
| // A custom host is often an ingestion proxy, which must not receive the secret key. | ||
| throw new EmailError({ | ||
| code: 'app_host_required', |
There was a problem hiding this comment.
P2 · Return the contractual missing_app_host error code
With a custom host and no appHost, send() returns app_host_required, but the cross-SDK contract in PostHog/posthog#120921 defines missing_app_host. Callers branching on that documented stable code miss this configuration error and fall through to generic error handling; return missing_app_host here.
There was a problem hiding this comment.
Fixed in cc3b6f6. A custom host without appHost now fails with missing_app_host, as PostHog/posthog#120921 defines. The test case is updated.
Node SDK compliance v2Commit: e6958e4 Capture v0 — acceptance✅ 28 passed / ❌ 3 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: Harness exit: 1; report validation exit: 1. Capture v1 — acceptance✅ 28 passed / ❌ 3 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: Harness exit: 1; report validation exit: 1. Capture v0 — migration✅ 55 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. Capture v1 — migration✅ 120 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. |
- A custom host without appHost now fails with missing_app_host, the code the cross-SDK contract in PostHog/posthog#120921 defines. - The request deadline uses a referenced timer, so a short-lived script can't exit while it awaits a send whose injected fetch holds no handle. - error.retryAfterMs keeps the full server Retry-After. parseRetryAfterMs in @posthog/core takes an optional cap for this; the 30 second retry ceiling still applies separately. - The email send responses entrance in Node.spec.md declares control: none with its reason, which closes the coherence spec gap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
Generated-By: PostHog Desktop Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
|
Size Change: +32.3 kB (+0.13%) Total Size: 25.2 MB 📦 View Changed
ℹ️ View Unchanged
|
posthog-js Compliance ReportDate: 2026-10-27 11:12:27 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
| case 422: | ||
| return 'invalid_idempotent_request' |
There was a problem hiding this comment.
P2 · Distinguish validation failures from idempotency errors on HTTP 422
For suppressed recipients or paused sending, the current template endpoint returns HTTP 422 with recipient_suppressed, sending_paused, or sending_suspended. emailErrorName labels all these as invalid_idempotent_request, so callers branching on the promised Resend-style names handle them incorrectly. Reserve that name for idempotency_key_reused and classify the other codes as validation errors.
There was a problem hiding this comment.
Fixed in d9eb1ef. On a 422, only idempotency_key_reused is now invalid_idempotent_request. recipient_suppressed, sending_paused and sending_suspended are validation_error, as PostHog/posthog#120921 names them. The name table test has a new 422 recipient_suppressed row. posthog-python gets the same change in PostHog/posthog-python@d2d4511.
| const response = await Promise.race([ | ||
| Promise.resolve().then(() => | ||
| this.hooks.fetch(url, { method: 'POST', headers, body, signal: controller.signal }) | ||
| ), | ||
| deadline, | ||
| ]) |
There was a problem hiding this comment.
P2 · Cancel response bodies that arrive after the request deadline
When an injected fetch ignores abort and resolves after the deadline, request() has already exited and discards the late response without consuming or cancelling its body. Repeated timeouts leave pooled connections occupied until upstream cleanup, exhausting transport resources. Attach late-response cancellation as the core transport already does.
There was a problem hiding this comment.
Fixed in d9eb1ef. When the deadline aborts the request, a response the injected fetch returns later has its body cancelled, as the core transport does. "times out a request that answers too late and releases the late response" covers this.
| throw invalid('invalid_max_retries', 'maxRetries must be a nonnegative integer.', 'maxRetries') | ||
| } | ||
|
|
||
| const payload: Record<string, unknown> = { to, message_data: templateData ?? {} } |
There was a problem hiding this comment.
P2 · Expose and forward the per-send engagement tracking control
For transactional emails that require tracking disabled, SendEmailOptions provides no captureEngagement option, and prepareSend drops it even when JavaScript callers pass false. The cross-SDK contract in PostHog/posthog#120921 requires this control on both forms; its absence prevents callers from disabling open and click capture. Add the option and serialize capture_engagement, defaulting to true.
There was a problem hiding this comment.
Not adding this yet. PostHog/posthog#120921 defines capture_engagement, but the template send endpoint this SDK calls (/messaging_templates/{id}/send/) does not accept it, so captureEngagement: false would be silently ignored and opens and clicks would still be captured. It can go in when the server supports it, which is planned for the single /messaging/send/email/ endpoint. posthog-python makes the same call.
…ponses - The template send route answers 422 for recipient_suppressed, sending_paused and sending_suspended as well as idempotency_key_reused. Only idempotency_key_reused is now invalid_idempotent_request; the send checks are validation_error, as the cross-SDK contract names them. - A response that an injected fetch returns after the deadline now has its body cancelled, as the core transport does, so repeated timeouts don't keep pooled connections busy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
A disabled client returned { data: { id: null }, error: null }, so a caller that treats a result without an error as sent dropped the email silently. It now sends nothing and returns the error client_disabled (name application_error), as the cross-SDK contract in PostHog/posthog#120921 defines. SendEmailResult.id is always the queued message ID, so its type is string.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-By: PostHog Desktop
Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
|
@PostHog review --deep |
There was a problem hiding this comment.
PostHog Review · Standard · reviewed b551aa4: 3 Should fix.
This is review round 3 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of 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.
| const text = await Promise.race([response.text(), deadline]) | ||
| return { status: response.status, payload: parseJson(text), retryAfterMs: readRetryAfter(response) } | ||
| } finally { | ||
| clearTimeout(timer!) |
There was a problem hiding this comment.
P2 · Cancel received response bodies when body reads time out
When an injected fetch returns headers before the deadline but its text() ignores abort and stalls, request() times out without cancelling the received body. The late-response callback has already run, so repeated retries leave connections occupied. Retain the response outside try and cancel its body on timeout in finally, as the core transport does.
There was a problem hiding this comment.
Fixed in d9cb1bf. request() now keeps the response outside the try and, when the deadline fires during the body read, cancels response.body without awaiting it. The new test "times out a response body that stalls and cancels it" covers a fetch whose text() never settles.
| const prepared = prepareSend(options, this.config.maxRetries) | ||
| if (this.hooks.isDisabled()) { | ||
| throw new EmailError({ | ||
| code: 'client_disabled', | ||
| message: 'The PostHog client is disabled, so no email was sent.', | ||
| }) |
There was a problem hiding this comment.
P2 · Reject email sends after client shutdown
After await posthog.shutdown(), a valid emails.send() still queues an email because send() checks only isDisabled(), and shutdown never sets that flag. This violates the shared email contract's client_closed behavior and permits email delivery after teardown. Track the client's closed state and return client_closed before issuing requests.
There was a problem hiding this comment.
Fixed in d9cb1bf. The client now records shutdown at the start of _shutdown() and passes an isClosed hook to EmailSender, so a send after shutdown() returns client_closed before any request. The new test "sends nothing after shutdown and returns client_closed" checks the code, the application_error name, and that fetch is not called.
| const headers: Record<string, string> = { | ||
| Authorization: `Bearer ${secretKey}`, | ||
| Accept: 'application/json', | ||
| 'Content-Type': 'application/json', | ||
| 'Idempotency-Key': prepared.idempotencyKey, | ||
| } |
There was a problem hiding this comment.
P2 · Forward the contractual migration attribution header
For apps migrating from another email provider, the shared contract requires migratedFrom and an X-PostHog-Migrated-From header on every email request. PostHogOptions exposes no such option, and deliver() never forwards it, so these sends lose migration attribution. Add the client option and propagate its value into the request headers.
There was a problem hiding this comment.
Declining for now. No PostHog server code reads X-PostHog-Migrated-From yet, and posthog-python does not send it either, so adding migratedFrom would publish an option that does nothing. We can add it in both SDKs when the server reads it.
…utdown When an injected fetch answers before the deadline but its body read stalls, the email request now cancels the received body after the timeout, so it frees its connection. After shutdown(), emails.send() now returns client_closed before any request, as the cross-SDK contract requires. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
There was a problem hiding this comment.
PostHog Review · Standard · reviewed d9cb1bf: 2 Should fix.
This is review round 4 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of 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.
| maxRetries: nonNegativeIntegerOr(this.options.emailRequestMaxRetries, DEFAULT_EMAIL_REQUEST_MAX_RETRIES), | ||
| }, | ||
| { | ||
| fetch: (url, fetchOptions) => this.fetch(url, fetchOptions), |
There was a problem hiding this comment.
P2 · Include active email sends in shutdown draining
When shutdown starts during an email request or retry backoff, it resolves without waiting because email sends bypass addPendingPromise and the queue that _shutdown() drains. A signal handler that exits after awaiting shutdown interrupts transactional sends despite an unexpired shutdown budget. Register each complete send promise, including retries, with the pending queue.
There was a problem hiding this comment.
Fixed in 3b21e02. EmailSender now tracks each send, retries included, and _shutdown() waits for them within the shutdown budget it already has. A send that starts after shutdown began still returns client_closed. I kept these sends out of addPendingPromise, because flush() (the automatic flush too) waits on that queue, so a send in its retry backoff would hold back event delivery. The new test "waits for a send in its retry backoff before shutdown resolves" covers a 503 followed by a retry.
| try { | ||
| const result = parseReply(await this.request(url, headers, prepared.body)) | ||
| for (const warning of result.warnings) { | ||
| this.hooks.logger.warn(`PostHog email warning ${warning.code}: ${warning.message}`) |
There was a problem hiding this comment.
P2 · Log email warnings outside debug mode
With debug mode off (the default), successful sends containing warnings never log them: deliver() calls the client's _logger.warn, whose logMsgIfDebug gate discards the message. Production logs therefore hide server guidance such as a superseded template while callers see a successful send. Route these warnings through an always-enabled warning hook.
There was a problem hiding this comment.
Declining. posthog-node logs only in debug mode by design, and this SDK keeps that policy. Callers get every warning in data.warnings on the result, and the template send endpoint this SDK calls returns no warnings today.
EmailSender now tracks each send, retries included, and shutdown() waits for those sends within its existing timeout. A send that starts after shutdown began still fails with client_closed. The sends are tracked apart from the client's pending-promise queue, because flush() waits on that queue and a send in its retry backoff must not hold back event delivery. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 3b21e02: 2 Should fix.
This is review round 5 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of 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.
| if (emailsDrained) { | ||
| await raceWithTimeout(emailsDrained, Math.max(0, shutdownDeadlineMs - Date.now())) |
There was a problem hiding this comment.
P2 · Drain emails concurrently with the core shutdown flush
With queued events, flushInterval: 0, and an email that outlasts the shutdown budget, this await consumes the entire deadline before super._shutdown() starts flushing analytics. Shutdown then resolves before the analytics request completes, losing those events when the caller exits. Run email draining concurrently with core shutdown under the shared deadline.
There was a problem hiding this comment.
Fixed in e6958e4. _shutdown() now starts the core shutdown flush at once and waits for the email drain next to it, both under the same deadline, so a slow send no longer delays queued events. The new test "flushes queued events while an email send is in its retry backoff" checks that the batch goes out during the backoff of a 503.
| response = await Promise.race([fetchPromise, deadline]) | ||
| const text = await Promise.race([response.text(), deadline]) | ||
| return { status: response.status, payload: parseJson(text), retryAfterMs: readRetryAfter(response) } |
There was a problem hiding this comment.
P2 · Preserve Retry-After when reading the response body fails
When a 429 or 503 response includes Retry-After but its body disconnects or times out, request() throws before reading the header. deliver() converts this to network_error and retries after about one second, even for a one-hour wait, increasing load during throttling or outages. Preserve the received retry delay when body reads fail.
There was a problem hiding this comment.
Fixed in e6958e4. When a non-2xx answer arrives but its body read fails or times out, request() now returns that status with a null payload and the parsed Retry-After, so the normal retry rules apply. A failed read on a 2xx stays a retryable network_error, and the body is still cancelled after the deadline. The new test "keeps the status and Retry-After when an error body cannot be read" covers a 429 with Retry-After: 120: it returns api_error with retryAfterMs 120000 after one fetch.
…try-After on a failed body read Shutdown now runs the core flush at once and waits for email sends in flight next to it, under the same deadline, so a slow send can no longer use up the budget for queued events. When the headers of an error answer arrive but the body read fails or times out, the send now keeps that status and its Retry-After, so the normal retry rules apply. A failed body read on a 2xx answer stays a retryable network error. The body is still cancelled when the read passes the deadline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: e72a2c10-f145-4f7c-8754-a4fcc06a18be
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
Re-add the stamphog label to request another review once you have addressed this.
stamphog declined to auto-review this pull request because of the size gate. It has 812 substantive lines in the global area, which is over the 800-line ceiling (8 files, or 1231 lines across 12 files once docs, generated files and snapshots are counted). The other gates (prerequisites, deny-list and tier) passed, so size is the only reason for the refusal.
You can ask a human reviewer to look at it. Or you can split it into smaller pull requests, for example moving the parseRetryAfterMs cap change in packages/core into its own PR, which should bring the remainder under the ceiling.
- posthog[bot] reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✗ | too large for auto-review (812L substantive in global — ceiling is 800L; 812L, 8F total, 1231L/12F incl. docs/generated/snapshots) |
| tier | ✓ | T1-agent / T1d-complex (1231L, 12F, two-areas, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ unknown · reviewed head e6958e4 |
Example
Problem
Server-side apps want to send PostHog transactional emails with one call on the client they already have, matching posthog-python.
This is the Node side of the template form in the cross-SDK contract PostHog/posthog#120921, and mirrors PostHog/posthog-python#1093. The code-content form,
emails.getand/messaging/send/email/are out of scope. The send API is in beta on the server.Changes
posthog.emails.send({ to, templateId, templateData?, distinctId?, idempotencyKey?, maxRetries? })posts to{appHost}/api/projects/@current/messaging_templates/{templateId}/send/withAuthorization: Bearer <secretKey>and anIdempotency-Key.templateDatabecomesmessage_dataanddistinctIdbecomesidentifiers.distinct_id. The request context is never used for the recipient.appHost,emailRequestTimeoutMs(default 15000) andemailRequestMaxRetries(default 2). The names follow the existingfeatureFlagsRequestTimeoutMs/featureFlagsRequestMaxRetriesoptions, like python'semail_request_*.secretKeyalready exists and is reused.us.i,eu.i,us,euandapp.posthog.com). For any otherhost, a send withoutappHostfails withmissing_app_hostbefore any request, because a custom host may be an ingestion proxy. A missing key fails withmissing_secret_key, also before any request.409 idempotency_key_in_flight,429 throttledand503are retried, with a backoff of about 1 s and then 2 s with jitter, orRetry-Afterwhen that is longer. ARetry-Afterover 30 s returns the error at once.429 quota_limitedand other 4xx responses are not retried.maxRetries: 0turns retries off.sendresolves to{ data, error }and does not reject.datais{ id, replayed, warnings }.erroris anEmailError. It carries PostHog'scodeas-is, so new server codes need no SDK change. It also carries a Resend-stylename,message,statusCode,retryAfterMs,field(fromattr), andwhy/fix/docsUrlwhen the server sends them.client_disabled, like python.Why
{ data, error }rather than throwingposthog-node's awaited request methods do not reject when a request fails.
getFeatureFlag,getFeatureFlagPayload,evaluateFlagsandgetRemoteConfigPayloadresolve withundefinedor a fallback and report the failure onposthog.on('error').PostHogFetchHttpErrorandPostHogFetchNetworkErrorare internal to core and never reach callers. Onlyflush()/shutdown()reject. No existing method returns structured error details to the caller.{ data, error }keeps the "resolve, don't reject" convention and gives the caller what it needs to branch, which is what the contract asks for Node. The error is returned, not also emitted onon('error'), so it is not reported twice.Differences from posthog-python#1093
{ data, error }instead of raising, as the contract specifies for Node.validation_errorwith codes such asinvalid_template_idorinvalid_idempotency_key. Python raisesTypeError/ValueErrorfor these.maxRetries, which the contract asks for.Emailsclass and noclient_closedstate. Node clients have no email transport to close and stay usable aftershutdown().Note
Divergence from PostHog/posthog#120921: the contract's
capture_engagementsend option is not here yet. The template send endpoint this SDK calls does not accept it, so the option would be silently ignored. It lands with the server support.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
claude-opus-5-5) in a PostHog Desktop task, directed by the assignee.src/__tests__/emails.spec.tsuses fake timers. It covers request shape, host derivation and the refused proxy host, a missing key, which responses are retried with the same key,Retry-After, the backoff timing, retry limits, the timeout, and argument checks.pnpm lint,pnpm check-types, the full posthog-node unit suite andtest:edge-compatpass. References were regenerated.🤖 Generated with Claude Code
Created with PostHog Desktop