Skip to content

fix: address code review findings#15

Merged
peter-svensson merged 2 commits intomainfrom
fix/code-review-findings
Apr 1, 2026
Merged

fix: address code review findings#15
peter-svensson merged 2 commits intomainfrom
fix/code-review-findings

Conversation

@peter-svensson
Copy link
Copy Markdown
Member

Summary

  • H1: Stop silently swallowing errors in stream ensure catch blocks. Now only NATS error codes 10058 (stream name already in use) and 10059 (consumer name exists) are suppressed; all other errors are rethrown.
  • H2: Rethrow errors when ephemeral consumer creation fails instead of silently dropping them.
  • H3: Replace 12-parameter positional signatures on startJSConsumers and startCoreConsumers with JSConsumerOptions and CoreConsumerOptions option interfaces.
  • M2: Replace non-null assertion (msg.headers!) with a guard check before calling extractToContext.
  • M3: Hoist TextEncoder and TextDecoder to module-level singletons instead of allocating per message.
  • M4: Validate that backOff array length does not exceed maxDeliver, throwing early with a clear message.

Test plan

  • bun run typecheck passes
  • All 76 tests pass (bun test)
  • CI passes

@peter-svensson peter-svensson enabled auto-merge (squash) April 1, 2026 19:23
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

- H1: Only suppress NATS error codes 10058/10059 in stream catch blocks,
  rethrow all other errors instead of silently swallowing them
- H2: Rethrow errors for ephemeral consumer creation instead of silently
  ignoring failures
- H3: Replace long positional parameter lists in startJSConsumers and
  startCoreConsumers with JSConsumerOptions/CoreConsumerOptions interfaces
- M2: Replace non-null assertion on msg.headers with guard pattern
- M3: Use module-level TextEncoder/TextDecoder instances instead of
  allocating per message
- M4: Validate that backOff array length does not exceed maxDeliver
@peter-svensson peter-svensson force-pushed the fix/code-review-findings branch from 5d9ea73 to fc1132d Compare April 1, 2026 19:30
@peter-svensson peter-svensson disabled auto-merge April 1, 2026 21:03
@peter-svensson peter-svensson merged commit 3645d6b into main Apr 1, 2026
4 checks passed
@peter-svensson peter-svensson deleted the fix/code-review-findings branch April 1, 2026 21:03
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