fix(history): return undefined when there is no history sync notification - #48
Conversation
…tion `getHistoryMsg` threw a Boom 400 for any message that is not a history sync — which is most messages. That breaks the shape a drop-in caller writes: `const h = getHistoryMsg(msg); if (!h) return` crashes instead of returning, and the caller has to know to wrap it. Absent is the ordinary case here, not an error, so it returns undefined and the return type says so. The compatibility test asserted the throw; it now asserts the value, and covers a message with no content and a missing argument as well. The registry entry that documented this is deleted — the deep target runs 5001 inputs with 0 findings and 0 excused after the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthrough
ChangesHistory message behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
getHistoryMsgthrew aBoom400 for any message that is not a history sync — which is most messages.That breaks the shape a drop-in caller writes:
Upstream returns
undefinedhere. Absent is the ordinary case for this helper, not an error condition: every message that is not a history sync takes that path.Change
Returns
undefined, and the return type says| undefinedso a caller who ignores it gets a type error rather than a runtime one. TheBoomimport is dropped along with the throw — it was the only use in the file.The compatibility test asserted the throw; it now asserts the value, and covers two cases it did not before: a message with no content at all, and a missing argument.
Registry
get-history-msg-throws-instead-of-undefinedis deleted. After the change the deeppure:getHistoryMsgtarget runs 5001 inputs with 0 findings and 0 excused, so the entry excuses nothing and an allowlist that outlives its divergence is how the same bug comes back. Open findings 22 → 21.Breaking change
Callers who relied on the throw to signal "not a history sync" — via
try/catch— stop seeing it. That behaviour was the defect, and no caller inside this repository depended on it:getHistoryMsghas no internal call sites, only the export and its test.Validation
tsc --noEmit,oxlint,oxfmt --checkclean.npm test1182 pass / 0 fail.npm run fuzz185 pass / 0 fail.🤖 Generated with Claude Code
https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
Generated by Claude Code
Summary by cubic
Return
undefinedfromgetHistoryMsgwhen a message has no history-sync notification, matching upstream and preventing crashes in common code paths. This fixes the drop-in pattern where callers check for absence instead of handling exceptions.Bug Fixes
getHistoryMsgreturnsundefinedinstead of throwing for non-history messages.Boomusage and updated tests (added no-content and missing-arg cases).Migration
undefinedcheck.Written for commit cb11209. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests