feat(emit): optional per-call signal + timeout cancellation (REVIEW P2)#2
Merged
Merged
Conversation
Resilience gap (REVIEW.md P2 "emit() cancellation"): emit() had no
per-call signal or timeout. If adapter readiness or post() hung, the
caller's emit() promise hung with it, forcing a bridge-wide reset() or
dispose() just to unstick one fire-and-forget event — coarse blast
radius for a single stuck call.
Add an OPTIONAL third argument `EmitOptions { signal?, timeoutMs? }`,
mirroring the existing call() cancellation conventions:
- signal is threaded into ready() so a hung adapter.ready() is abortable,
and races adapter.post() so a hung post() (e.g. a Flutter callHandler
that never settles) rejects promptly with the signal's reason.
- positive timeoutMs bounds the readiness-wait + post() window and rejects
with BridgeTimeoutError; timeoutMs <= 0 disables the timer, matching
call()'s BRG-R-02 contract (pair with signal if the adapter may hang).
- a pre-aborted signal short-circuits before any adapter side effect.
- timer and abort listener are torn down on every settle path, leaving no
orphaned timer and no listener pinned on the signal.
Additive and backwards-compatible: with no option, emit() keeps its exact
pre-existing contract (a bare awaited post(), no timer) via an explicit
fast path. Per-call timeout is opt-in only — unlike call(), emit() does
NOT fold in the bridge default timeout, so today's unbounded emit() is
unchanged.
No cross-package imports; no provider-specific leakage. New public type
EmitOptions is exported from the root subpath.
Tests (RED-first): signal abort mid-flight + listener teardown, per-call
timeout + timer-clear-on-success, timeoutMs<=0 disable, pre-aborted
signal (no post), abort during readiness (no post), and a regression that
no-option emit() arms no timer and posts exactly once.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds opt-in per-call cancellation and timeouts to bridge.emit(), bringing emit() closer to call()’s resilience story while preserving the existing no-options “fire-and-forget” behavior.
Changes:
- Adds
EmitOptions(signal,timeoutMs) and extendsBridge.emit(event, payload?, options?). - Implements per-call abort/timeout handling in
emit()(with a no-options fast path to preserve current behavior). - Adds a focused test suite covering abort/timeout behavior for
emit().
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/bridge.ts |
Extends emit() to accept EmitOptions and races post() against per-call abort/timeout. |
src/types.ts |
Introduces exported EmitOptions and updates the Bridge.emit() type signature. |
src/index.ts |
Re-exports EmitOptions from the public root entrypoint. |
test/bridge.test.ts |
Adds new tests for emit() abort and timeout semantics plus regression coverage for the no-options path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+411
to
+418
| if (signal) { | ||
| abortHandler = (): void => { | ||
| settleReject(signal.reason); | ||
| }; | ||
| signal.addEventListener("abort", abortHandler, { once: true }); | ||
| } | ||
|
|
||
| adapter.post(envelope).then(settleResolve, settleReject); |
Comment on lines
339
to
+343
| const capturedEpoch = resetEpoch; | ||
| await ready(); | ||
| // Thread the signal through readiness so a hung adapter.ready() can be | ||
| // unstuck by this single emit's abort, not only by reset()/dispose(). | ||
| await (signal !== undefined ? ready({ signal }) : ready()); | ||
|
|
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.
Resilience gap (REVIEW.md P2 — `emit()` cancellation, Open)
The README already flagged the asymmetry: "`call()` supports per-call `signal` and `timeoutMs`. `emit()` does not." A single stuck fire-and-forget event forced a bridge-wide `reset()`/`dispose()` — coarse blast radius.
Change (additive, backwards-compatible)
New optional third argument to `emit()`, a dedicated `EmitOptions` interface mirroring `CallOptions`:
```ts
emit(event: string, payload?: unknown, options?: EmitOptions): Promise
interface EmitOptions {
timeoutMs?: number; // opt-in; positive bounds ready()+post(), <=0 disables (mirrors call() BRG-R-02)
signal?: AbortSignal; // aborts the readiness wait or post(); rejects with signal.reason
}
```
Mirrors the existing `call()` cancellation conventions exactly:
Backwards-compat: with no option, `emit()` keeps its exact pre-existing contract (a bare awaited `post()`, no timer) via an explicit fast path. Per-call timeout is opt-in only — unlike `call()`, `emit()` does NOT fold in the bridge default timeout, so today's unbounded `emit()` is unchanged.
No cross-package imports; no provider-specific leakage. `EmitOptions` exported from the root subpath.
RED → GREEN evidence
6 new-API tests authored before implementation, confirmed failing against the old `emit(event, payload?)`:
```
RED: Tests 6 failed | 2 passed (E4 resolved instead of rejecting; E1/E2/E5 timed out — option ignored)
GREEN: Tests 8 passed
Full suite: 145 passed (was 137)
```
Tests added (`test/bridge.test.ts`):
Gate — `pnpm prepublishOnly` passes
🤖 Generated with Claude Code