Skip to content

fix(core): isolate internal/status observers - #55

Open
tdwhere123 wants to merge 1 commit into
cordiverse:mainfrom
tdwhere123:fix/core-isolate-internal-status
Open

fix(core): isolate internal/status observers#55
tdwhere123 wants to merge 1 commit into
cordiverse:mainfrom
tdwhere123:fix/core-isolate-internal-status

Conversation

@tdwhere123

Copy link
Copy Markdown

Problem

Fiber._updateState emits internal/status with no isolation. A diagnostic observer that throws is caught by the Fiber constructor, written into _error, and can reject await plugin even when the plugin itself succeeded. The same throw also:

  • overwrites a real plugin execution error, so FAILED is attributed to the observer
  • aborts remaining internal/status listeners
  • skips the reflect.notify that follows a transition into or out of ACTIVE
  • turns a later _reload emit into an unhandled rejection when nobody awaits the Fiber

internal/plugin disposal observers already follow a log-only boundary (see #39). internal/status on current main does not.

Observation Current main This change
healthy plugin + throwing observer await plugin rejects with the observer error Fiber stays ACTIVE; observer is logged
plugin throw + throwing observer await plugin rejects with the observer error rejects with the plugin error; state is FAILED
two status observers, first throws second observer never runs second observer still runs
observer returns a rejected thenable unhandled rejection logged; Fiber stays ACTIVE

The five regressions fail on 8cc9e33 for those reasons and pass on this branch.

Change

Replace the unguarded emit('internal/status') with a local dispatchFiberStatus helper:

  • resolve listeners through the existing events.dispatch path so thisArg/bind semantics stay the same
  • catch each listener independently and report via ctx.logger.error
  • observe thenable rejections without awaiting them
  • leave reflect.notify on the success path after the broadcast

This does not change internal/plugin, internal/dispatch, or the Fiber generation/epoch machinery in #39 / #54.

Regression coverage

packages/core/tests/status.spec.ts covers the five rows above, including a provider/consumer case so a throwing observer cannot skip service notify.

Validation

  • yarn lint
  • yarn yakumo tsc
  • yarn test cordis/status — 5 tests passed
  • yarn test cordis — 13 files, 76 tests passed
  • yarn test:json — 20 files, 168 tests passed
  • git diff --check origin/main...HEAD — passed

Observer failures were poisoning Fiber._error and aborting sibling listeners plus service notify. Contain each listener independently, matching the internal/plugin disposal isolation pattern.
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.

1 participant