Stop a host's callback from breaking the guard that calls it

[patchstackdave]

ENG-3648

createProtection takes three callbacks from the host — onError, onDetect, onSkip. Only onSkip was wrapped so a throw could not escape, with the reason written beside it:

onSkip … a reporting callback must never affect request handling

The other two were unguarded, across 23 call sites. This is not a new rule; it is that rule applied where it was missing.

onDetect is the sharp one — it runs on the request path

Verified directly rather than reasoned about: a handler that throws propagates out of the fetch guard and the request fails. And it fires only when a rule matched, so a bug in the host's logging callback turns "we noticed something" into "the app broke", on precisely the requests that mattered. That is the inverse of the single promise this package makes.

onError costs boots and refreshes

resolveRules reports a recovered condition through onError — the rule fetch failed, falling back to cached or bundled rules. Handing that to a broken handler aborted createProtection, so an app lost protection entirely over a bug in its own logging. On the refresh path the same throw becomes an unhandled rejection inside a poll loop, which kills a long-lived process long after the mistake was made.

Found while writing #158's test: an assertion about that diagnostic being contained failed for an unrelated reason, which was this. I removed my one-off guard there rather than ship it, because a single wrapped call site implies a containment the other 22 don't have.

Shape

One notify(fn, arg, label) helper at every site. It returns whether the callback ran, and the engine uses that: a host handler which runs takes over error reporting, one which throws falls through to the built-in report-once logging — otherwise a rule error would vanish between two broken reporters.

Not silent. A throwing hook is a bug in the host's code, so the first failure per hook is reported — once per process, because these run per request and the alternative is an unbounded log flood.

Tests

tests/protect/callback-containment.test.ts, 9 cases. Every containment test is paired with a control, because "contained" has a cheap wrong implementation: never call the callback at all.

• serves the request when onDetect throws, and is not wedged for later requests
• still blocks when onDetect throws — containment must not become a bypass; a reporting callback is downstream of the decision, so a broken one costs the report, never the enforcement
• still calls a callback that works — the control for both of the above
• boots when onError throws; refreshes when onError throws
• onSkip still contained (locking pre-existing behaviour while generalising the rule it demonstrated)
• notify reports delivery accurately, warns once rather than per call, and survives a runtime with no console

Mutation-checked, four ways: letting the throw escape fails seven; never calling the callback fails only the controls; warning per failure fails only the warn-once test; always claiming delivery fails only the fall-through test.

Full suite green (1276 passed, 6 skipped), typecheck and template typecheck clean.

[coderbuds]

Comprehensive callback-containment feature correctly wraps host callbacks contextually.

🎯 Quality: 100% Elite · 📦 Size: Medium

📈 This month: Your 109th PR — above team average · Averaging Excellent

_{See how your team is trending →}

[patchstackdave]

/review