feat(email): per-template render-error policy (fail/degrade/fallback) - #57
Merged
Conversation
A conditional rule that *throws* at render (not a non-match) now drives a per-template `EmailErrorPolicy` (`onError`, default `fail`): - evaluator surfaces render-time throws via an `onError` sink (fires only on a rule throw, never a non-match); inline error comments now gate on EMAIL_INLINE_RENDER_ERRORS (decoupled from env, so testable) instead of isTest - composeTemplate returns the resolved ownerModel + onError; parentOwner walks Space → Org → default - sendEmail render loop: clean batch sends; on a throw it always logs, then degrade (drop the throwing block) / fallback (re-compose one owner up, threading ...ctx) / fail (EmailRenderError → BullMQ retries → DLQ). Base owners always fail - save-time validates conditionals in both mjml and subject; saveComponent validates conditionals at the unit boundary (MJML fragments aren't doc-validated) Per-tenant owner resolution is still stubbed to default, so the policy resolves to fail until that lands; the machinery + seam ship now. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pre-existing repo debt unrelated to the email feature, surfaced while validating: - biome 2.4.6→2.5.0 drift: lockfile pins 2.5.0 but configs used the deprecated `recommended` field. `biome migrate` updates all three biome.json to `preset` - exclude `**/public` from lint (noSvgWithoutTitle fired on Next boilerplate SVGs) - allow default export in `.atlas/config.ts` (config file) + reformat it - regenerate the stale `packages/ui/src/apiClient` barrel — it was missing Contact endpoints from a prior PR, failing the monorepo typecheck Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…any) Adversarial review (#1): the fallback re-compose could query with an undefined organizationId/spaceId, which Prisma treats as "no filter" → match-any across tenants. Latent today (sendEmail starts at default → fallback unreachable) but a footgun once per-tenant resolution lands. - lookupAtSpace/lookupAtOrg coalesce a missing tenant id to null (no match at this tier → cascade down) instead of undefined (drops the filter) - sendEmail renderCtx is now the explicit per-send tenant context, reused for the initial compose and every fallback re-compose so scoping is preserved up the cascade - regression test: an Org lookup with no organizationId throws rather than leaking another tenant's template Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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.
What
Adds a per-template render-error policy for emails: when a conditional rule throws at render time (a malformed/uncheckable rule — not a normal non-match), the template's
onErrorpolicy decides what happens.EmailTemplate.onError: EmailErrorPolicy(defaultfail):fail— throwEmailRenderError→ BullMQ retries → DLQ-equivalent (attempts: 3,removeOnFail: { age: 30d }). Safest.degrade— send the batch with the throwing block(s) dropped (the evaluator already excludes them).fallback— re-compose one owner up (parentOwner: Space → Org → default) and re-render; bounded, loops until clean or a base owner forcesfail.Base owners (
default/admin) aren't author-configurable and have no parent → alwaysfail.How
evaluateConditions/interpolate) takes an optionalonError: RuleErrorSink, firing only on a rule throw, never on a non-match. The dev-only inline<!-- RULE ERROR -->comment is decoupled fromisTestand now gates onEMAIL_INLINE_RENDER_ERRORS— observability (logging) is env-independent; the artifact-mutation is an explicit, testable debug flag.composeTemplatereturns the resolvedownerModel+onError;parentOwneris the cascade helper.sendEmailrender loop collects throws viaonError, always logs (LogScope.email), then applies the policy.fallbackthreads...ctxso per-tenant context flows through the cascade once it lands.mjmlandsubject;saveComponentvalidates conditionals at the unit boundary (component MJML is a fragment, so it isn't run through the full-document MJML validator).Tests
evaluateConditionsonError + inline-flag,parentOwner, compose result fields, rewritten interpolate render-error tests.bun run checkgreen (lint, typecheck, 776 backend tests, frontend, CI rules).Second commit —
chore: get check greenPre-existing repo debt, unrelated to the feature, fixed to get
bun run checkpassing (it was already red in-tree):2.4.6→2.5.0config migration (lockfile pins 2.5.0; configs used the deprecatedrecommendedfield)**/publicfrom lint (noSvgWithoutTitleon Next boilerplate SVGs).atlas/config.ts+ reformatpackages/ui/src/apiClientbarrel (was missingContactendpoints from a prior PR → monorepo typecheck failure)These are repo-wide lint/toolchain changes — worth a look to confirm you're happy with them as committed.
🤖 Generated with Claude Code