Skip to content

fix(forward-email): keep applying a domain's alias writes after one fails - #3

Merged
srgvg merged 1 commit into
mainfrom
fix/2-apply-abort-scope
Aug 31, 2026
Merged

fix(forward-email): keep applying a domain's alias writes after one fails#3
srgvg merged 1 commit into
mainfrom
fix/2-apply-abort-scope

Conversation

@srgvg

@srgvg srgvg commented Aug 30, 2026

Copy link
Copy Markdown
Member

Fixes #2.

  • _apply wraps each alias create/update/delete in its own ForwardEmailApiError handler: the error is printed for that alias, the domain's remaining writes still run, and the run exits 1.
  • Plan-time WARNING when a planned create/update touches an alias whose regex name contains Perl-style look-around ((?!, (?=, (?<) — FE re-validates the stored pattern on every write and rejects these, even for a quota-only update with no name in the body (probed live, see the issue). A look-around alias with no planned change stays silent.
  • README: contract bullet documenting the grandfathered-pattern trap and the new abort scope.

Tests: 3 new (failing-alias-continues, warning-on-planned-change, silent-when-clean), written first and watched fail; suite 448 -> 451 green.

…ails

One failing alias write (create/update/delete raising ForwardEmailApiError)
no longer aborts the domain's remaining, independent writes: the error is
printed per alias and the run still exits 1. Found live (issue #2): Forward
Email re-validates an alias's stored regex pattern on every write and now
rejects Perl-style look-around, so a grandfathered `(?!...)` alias 400s on a
quota-only update — and took its domain's other pending update down with it.

The pattern trap itself is FE-side and unfixable from the client (probed:
omitting `name` from the update body still 400s), so surface it early
instead: plan prints a WARNING when a planned create/update touches an alias
whose name contains look-around, and the README documents the contract.

Closes #2.
@srgvg

srgvg commented Aug 30, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-31T04:56:55.630984Z 8413861 Draft marked ready
ℹ️ 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" or "@codex security review".

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

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 841386109e

ℹ️ 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".

@srgvg
srgvg marked this pull request as ready for review August 31, 2026 04:54
@srgvg
srgvg merged commit c079bc1 into main Aug 31, 2026
10 checks passed

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 841386109e

ℹ️ 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".

ok = False
continue
out.write(f" applied alias {chg.action} {chg.name}\n")
for chg in deletes:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Do not prune aliases after a replacement create fails

With PRUNE=1, a rename or migration is planned as a create for the new alias plus a delete for the old alias. If that create raises ForwardEmailApiError, ok becomes false but this loop still deletes the old, working alias, leaving the address with no delivery target; previously the exception reached the outer handler before any deletes ran. Continue attempting the remaining creates/updates, but skip the delete phase if any of those writes failed.

Useful? React with 👍 / 👎.

@srgvg
srgvg deleted the fix/2-apply-abort-scope branch August 31, 2026 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

forward-email: one failing alias update aborts the rest of the domain's alias changes

1 participant