Skip to content

chore: initial review#1

Merged
Marsup merged 5 commits intomainfrom
chore/initial-review
Apr 7, 2026
Merged

chore: initial review#1
Marsup merged 5 commits intomainfrom
chore/initial-review

Conversation

@Marsup
Copy link
Copy Markdown
Contributor

@Marsup Marsup commented Mar 19, 2026

Here is a 1st pass review for the initial implementation.

Coverage & Testing

  • Reached 100% coverage and updated vitest configuration to enforce 100% coverage thresholds and do typecheck
  • Migrated tests from src/ to a dedicated test/ directory
  • Updated vitest to the latest version
  • Parallelized all the tests to be as fast as possible

Build & CI

  • Updated typescript tsdown
  • Switched import extensions from .ts to .js in source files
  • Refined tsconfig.json and tsdown.config.ts to use isolatedDeclarations, verbatimModuleSyntax, and stripInternal for cleaner builds (the exported types contained private fields which made the typings unusable)
  • Changed target to node 22+

@Marsup Marsup requested a review from damusix March 19, 2026 14:55
@Marsup Marsup force-pushed the chore/initial-review branch from ec4593a to a06b7d5 Compare March 19, 2026 15:25
damusix and others added 2 commits April 1, 2026 01:26
- Enforce retry floor (min 1000ms) to prevent reconnection storm DoS
- Sanitize Last-Event-ID header by stripping control characters (\x00-\x1f)
- Add maxSessions per-subscription connection limit with 503 rejection
- Add maxDuration connection TTL with ±10% jitter to prevent thundering herd
- Add 29 security tests covering CRLF injection, retry abuse, cross-client isolation, connection exhaustion, and post-stop safety
- Document new options and security properties in API.md
damusix and others added 2 commits April 7, 2026 12:56
Surface mistakes in configuration up-front (at register, subscription
declaration, route build, and replayer construction) instead of letting
them fail silently or surface deep inside request handling.

- Plugin options validated against a labeled Joi schema at register()
  covering keepAlive, retry, headers, hooks, and backpressure.
- subscription() asserts the path via Hoek.assert (non-empty, starts
  with "/") and validates SubscriptionConfig — filter/onSubscribe/
  onUnsubscribe/onReconnect, retry, keepAlive, replay (duck-typed
  Replayer), maxSessions, maxDuration.
- The sse handler decoration validates SseHandlerOptions once when the
  route builds the handler, embedding "METHOD path" in the error so the
  failing route is obvious.
- FiniteReplayer and ValidReplayer constructors validate their own
  option objects.

Hot paths (publish, broadcast, eachSession, Session.push) are
deliberately untouched — validation only runs at configuration
boundaries that are exercised once per server lifecycle.

Adds joi and @hapi/hoek as direct dependencies.
@Marsup Marsup merged commit 93a968e into main Apr 7, 2026
3 checks passed
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.

2 participants