Repository navigation
fix(api): sequences stall on wrapped footer address; render failures retry forever - #155
Merged
Merged
Conversation
Image tags now keep the leading v (v0.1.0, not 0.1.0), matching Frontlit, so IMAGE_TAG for infra's deploy.sh is the release tag across apps. Drops the major.minor tag; latest still tracks the newest release. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…der retries The managed-footer compliance check compared each mailing-address line to the raw rendered text. The footer renderer wraps long single-line addresses with a newline and indentation, so the check threw footer_render_failed and no email in the sequence could send. Collapse whitespace on both sides before comparing. A render failure also happened before attemptMailSending's try block, so it never incremented retry_count and the enrollment retried every ~30s forever. Extract the retry/drop logic into recordFailedAttempt and use it for render failures too, so they stop at sequenceBounceLimit and appear in report.failed. Fixes #154 Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e9e44b9c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A render failure (e.g. the managed footer check) is permanent for this | ||
| // recipient until the content or settings change. Count it like any other | ||
| // failed attempt so it eventually stops instead of retrying forever. | ||
| await recordFailedAttempt({ sequence, contact, ongoingSequence }); |
There was a problem hiding this comment.
If renderCampaignHtml fails because a shared dependency is temporarily unavailable—for example, its getTeamSettingsRow or cloud getOrganizationEntitlements database reads—this increments the recipient's retry count just like invalid content. After the bounce limit, recordFailedAttempt deletes the enrollment, so a brief outage can permanently skip recipients even after service recovers. Only terminal content or configuration errors should consume this per-recipient retry limit.
Useful? React with 👍 / 👎.
Address review feedback: a transient dependency error while rendering (for example a database read for team settings or entitlements) must not consume the per-recipient retry limit, or a brief outage would permanently drop enrollments. Throw a typed EmailRenderError for the footer/render failures and count only those, TemplateValidationError and MissingTemplateVariablesError. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
…ing recipients Counting render failures per recipient meant a team-wide problem (an empty mailing address, or a footer render bug like #154) unenrolled every recipient within minutes. Content and footer errors (EmailRenderError, TemplateValidationError, Liquid syntax errors) now auto-pause the broadcast or sequence with a new `render_error` reason: the team is notified, everyone stays queued, and resuming sends to all of them. Other errors while rendering are transient and are retried without counting. The send-failure path is back to exactly what it was on main. The pause banner and the owner email explain the reason in words, and SequenceReport.autoPause.reason in @sendlit/email-blocks gains `render_error`. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… error Review follow-ups to the render_error auto-pause: - Only Liquid syntax errors (ParseError, TokenizationError) pause the send. A Liquid RenderError depends on one contact's data, so that recipient's email is skipped and recorded in report.sequence.failed (with an atomic jsonb append), like a suppressed recipient, and everyone else still gets it. Previously one such contact paused the whole broadcast on every resume. - If the pause doesn't take effect (the sequence is neither active nor paused), rethrow the original error instead of swallowing it. - A broadcast auto-paused because its email couldn't be built shows its "Edit content" link, so the content can be fixed before resuming. Its audience, schedule and send actions stay locked. - Pause wording names the actual cause: a missing footer block is a content problem, not a mailing-address one. - The @sendlit/email-blocks changeset is minor: widening a public union can break exhaustive switches. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…pping everyone
A Liquid RenderError can come from the content itself (for example
`{% include header %}`), not only from one contact's data. Treating every
RenderError as per-recipient would mark the whole audience failed and
complete a broadcast with nothing sent. Before skipping a recipient, render
the email once more without any contact data (the public web version's
context); if that also fails, the error is content-wide and the send pauses
like any other render_error.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The render-without-contact-data probe can't see a content error behind a
condition on contact data, such as
`{% if subscriber.name %}{% include x %}{% endif %}`: every named contact
would be skipped. Count recipient skips on the existing consecutive-failure
streak (cleared by a successful send) and, at AUTO_PAUSE_CONSECUTIVE_FAILURES,
pause with render_error and keep the recipient queued instead of skipping.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Resolve the conflict in process-ongoing-sequence.ts by keeping the render error handling and main's new viewInBrowser, event and eventId render arguments. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #154. Follow-up work is tracked in #156.
Problem
An active "new contact subscribes" sequence sent nothing in production (Sent 0, Recipients 0). Its enrollment failed with
footer_render_failedevery ~30s.mailingAddressAppearsInTextcompared each address line with the rawtextContentof the rendered footer. The renderer wraps a long single-line address with a newline and indentation, so a valid address never matched and every send for the team was rejected.Changes
Footer check (
mail/render.ts): collapse whitespace on both sides before comparing. Footer/render failures now throw a typedEmailRenderError(same messages).Render failures (
automation/process-ongoing-sequence.ts), classified like other email platforms do:EmailRenderError), invalid content (TemplateValidationError), Liquid syntax (ParseError/TokenizationError)render_error. Owners are emailed, every recipient stays queued, nothing is dropped, and Resume sends to everyone.RenderError) that also fails without any contact data (e.g.{% include x %})report.sequence.failed(atomic jsonb append), like a suppressed recipient. Everyone else still gets it.AUTO_PAUSE_CONSECUTIVE_FAILURES, default 5)Handling of failures after the email is built (SMTP/ESP errors) is unchanged from
main.UI and types
render_errorin plain words (e.g. "The workspace has no mailing address for the footer").render_errorshows "Edit content" so it can be fixed before resuming; its audience, schedule and send actions stay locked.@sendlit/email-blocks:SequenceReport.autoPause.reasongainsrender_error(minor changeset).Testing
apps/api: typecheck and lint clean;apps/webtypecheck clean.sending-health.test.ts: 10/10. The new tests cover the footer-settings pause (queue survives, resume sends to all, broadcast completes), transient retry, Liquid syntax pause, content-wide Liquid runtime pause, a per-contact skip, the consecutive-skip backstop and the wording. Each behaviour test fails without its fix.main: 845 pass, 0 fail in a clean checkout, and CI (quality,test,verify) is green.render_error: shows "Edit content" with the audience locked. A bounce-rate pause shows no edit link.Known gap
A content error behind a condition on contact data (e.g.
{% if subscriber.name %}{% include x %}{% endif %}) is still skipped per recipient without pausing, if those recipients are interleaved with contacts that send fine. Those recipients are recorded inreport.sequence.failed. The real fix is rejecting such Liquid when content is saved, tracked in #156.Workaround until deployed
A mailing address of about 55 characters or less renders without wrapping.
🤖 Generated with Claude Code