Repository navigation
🐛 fix(watch): count onEvent sink failures as emitErrors, not classifyErrors - #390
Conversation
…rors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Read watch() in full at 69d3c45 (src/watch.ts:75-192) plus the diagnostics consumers. Looks correct to me.
emitnow owns theonEventtry/catch (src/watch.ts:87-92), so in stream mode the outer handler'scatch(src/watch.ts:165-168) only seesstripANSI/classifyfailures, and in capture mode the poll loop'scatch(src/watch.ts:124-128) only sees tmux/classifier failures. The misattribution in #388 is gone in both modes.- Small behavioural side effect, which I think is an improvement: a throwing sink no longer aborts the rest of the line/frame handler, so
includeRawoutput (src/watch.ts:164) is still emitted after a failed classified-event delivery. - Diagnostics spreads the stats object, so
emitErrorsreaches the stderr summary without further wiring; README sample and runbook row updated to match. - Built and ran
test/diagnostics.test.jslocally: 18/18 pass, including the new classifier-failure counter test.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read src/watch.ts at the PR head rather than just the diff. Looks correct to me.
emit(src/watch.ts:84-91) is the single funnel for both stream and capture mode, so wrappingopts.onEventthere is the right place: a throwing sink now incrementsemitErrorsand no longer propagates into the stream-modecatchatsrc/watch.ts:164-167(which only countsclassifyErrors) or the capture-modecatchatsrc/watch.ts:132-136(captureFailures). Both of those previously misattributed sink failures.src/cli.ts:219spreadswatcher.stats()straight into the diagnostics line, so the new counter reaches--diagnosticsoutput without further wiring; README sample and the runbook table (runbooks/session-monitoring-failure.md:50) are updated to match.- The new
Classifier.prototype.classifystub test pins the remainingclassifyErrorspath, so the two counters are now independently tested.
One behaviour change worth knowing, not a defect: in stream mode with --include-raw, a sink that throws on the classified event previously aborted the line (the raw emit at src/watch.ts:162 was skipped). It now still emits the raw event, so a flaky sink sees more deliveries than before. That seems like the intended semantics — the sink failing is the sink's problem — but noting it since it changes observable output.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
watch wrapped both classification and the onEvent callback in one try, so a throwing sink was counted as classifyErrors, misdirecting the diagnostics runbook. onEvent is now guarded inside emit and counted in a new emitErrors counter (also covers capture mode); classifyErrors only reflects classifier failures. Adds tests, a runbook row, README sample update and a changelog fragment. Note: eventsEmitted still counts events the sink threw on (documented on the field).
Closes #388
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88