Repository navigation
fix(server): Fastify 5 modernisation, fetch polyfill removal, and test coverage - #160
Merged
Merged
Conversation
Bumps and [brace-expansion](https://github.com/juliangruber/brace-expansion). These dependencies needed to be updated together. Updates `brace-expansion` from 1.1.11 to 1.1.12 - [Release notes](https://github.com/juliangruber/brace-expansion/releases) - [Commits](juliangruber/brace-expansion@1.1.11...v1.1.12) Updates `brace-expansion` from 2.0.1 to 2.0.2 - [Release notes](https://github.com/juliangruber/brace-expansion/releases) - [Commits](juliangruber/brace-expansion@1.1.11...v1.1.12) --- updated-dependencies: - dependency-name: brace-expansion dependency-version: 1.1.12 dependency-type: indirect - dependency-name: brace-expansion dependency-version: 2.0.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [pbkdf2](https://github.com/browserify/pbkdf2) from 3.1.2 to 3.1.5. - [Changelog](https://github.com/browserify/pbkdf2/blob/master/CHANGELOG.md) - [Commits](browserify/pbkdf2@v3.1.2...v3.1.5) --- updated-dependencies: - dependency-name: pbkdf2 dependency-version: 3.1.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [serialize-javascript](https://github.com/yahoo/serialize-javascript) to 6.0.2 and updates ancestor dependency [mocha](https://github.com/mochajs/mocha). These dependencies need to be updated together. Updates `serialize-javascript` from 6.0.0 to 6.0.2 - [Release notes](https://github.com/yahoo/serialize-javascript/releases) - [Commits](yahoo/serialize-javascript@v6.0.0...v6.0.2) Updates `mocha` from 10.0.0 to 10.8.2 - [Release notes](https://github.com/mochajs/mocha/releases) - [Changelog](https://github.com/mochajs/mocha/blob/main/CHANGELOG.md) - [Commits](mochajs/mocha@v10.0.0...v10.8.2) --- updated-dependencies: - dependency-name: serialize-javascript dependency-version: 6.0.2 dependency-type: indirect - dependency-name: mocha dependency-version: 10.8.2 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [base-x](https://github.com/cryptocoinjs/base-x) from 3.0.9 to 3.0.11. - [Commits](cryptocoinjs/base-x@v3.0.9...v3.0.11) --- updated-dependencies: - dependency-name: base-x dependency-version: 3.0.11 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [sha.js](https://github.com/crypto-browserify/sha.js) from 2.4.11 to 2.4.12. - [Changelog](https://github.com/browserify/sha.js/blob/master/CHANGELOG.md) - [Commits](browserify/sha.js@v2.4.11...v2.4.12) --- updated-dependencies: - dependency-name: sha.js dependency-version: 2.4.12 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [cipher-base](https://github.com/crypto-browserify/cipher-base) from 1.0.4 to 1.0.7. - [Changelog](https://github.com/browserify/cipher-base/blob/master/CHANGELOG.md) - [Commits](browserify/cipher-base@v1.0.4...v1.0.7) --- updated-dependencies: - dependency-name: cipher-base dependency-version: 1.0.7 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [tmp](https://github.com/raszi/node-tmp) from 0.2.1 to 0.2.5. - [Changelog](https://github.com/raszi/node-tmp/blob/master/CHANGELOG.md) - [Commits](raszi/node-tmp@v0.2.1...v0.2.5) --- updated-dependencies: - dependency-name: tmp dependency-version: 0.2.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [lodash](https://github.com/lodash/lodash) from 4.17.21 to 4.17.23. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.21...4.17.23) --- updated-dependencies: - dependency-name: lodash dependency-version: 4.17.23 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps and [semver](https://github.com/npm/node-semver). These dependencies needed to be updated together. Updates `semver` from 6.3.0 to 6.3.1 - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v6.3.1/CHANGELOG.md) - [Commits](npm/node-semver@v6.3.0...v6.3.1) Updates `semver` from 7.3.5 to 7.7.3 - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v6.3.1/CHANGELOG.md) - [Commits](npm/node-semver@v6.3.0...v6.3.1) Updates `semver` from 5.7.1 to 5.7.2 - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v6.3.1/CHANGELOG.md) - [Commits](npm/node-semver@v6.3.0...v6.3.1) --- updated-dependencies: - dependency-name: semver dependency-version: 6.3.1 dependency-type: indirect - dependency-name: semver dependency-version: 7.7.3 dependency-type: indirect - dependency-name: semver dependency-version: 5.7.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [qs](https://github.com/ljharb/qs) and [body-parser](https://github.com/expressjs/body-parser). These dependencies needed to be updated together. Updates `qs` from 6.10.3 to 6.14.2 - [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md) - [Commits](ljharb/qs@v6.10.3...v6.14.2) Updates `body-parser` from 1.20.0 to 1.20.4 - [Release notes](https://github.com/expressjs/body-parser/releases) - [Changelog](https://github.com/expressjs/body-parser/blob/master/HISTORY.md) - [Commits](expressjs/body-parser@1.20.0...1.20.4) --- updated-dependencies: - dependency-name: qs dependency-version: 6.14.2 dependency-type: indirect - dependency-name: body-parser dependency-version: 1.20.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [ajv](https://github.com/ajv-validator/ajv) to 8.18.0 and updates ancestor dependency [fastify](https://github.com/fastify/fastify). These dependencies need to be updated together. Updates `ajv` from 6.12.6 to 8.18.0 - [Release notes](https://github.com/ajv-validator/ajv/releases) - [Commits](ajv-validator/ajv@v6.12.6...v8.18.0) Updates `fastify` from 3.27.4 to 5.7.4 - [Release notes](https://github.com/fastify/fastify/releases) - [Commits](fastify/fastify@v3.27.4...v5.7.4) --- updated-dependencies: - dependency-name: ajv dependency-version: 8.18.0 dependency-type: indirect - dependency-name: fastify dependency-version: 5.7.4 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [tar](https://github.com/isaacs/node-tar) to 7.5.9 and updates ancestor dependency [@mapbox/node-pre-gyp](https://github.com/mapbox/node-pre-gyp). These dependencies need to be updated together. Updates `tar` from 6.1.11 to 7.5.9 - [Release notes](https://github.com/isaacs/node-tar/releases) - [Changelog](https://github.com/isaacs/node-tar/blob/main/CHANGELOG.md) - [Commits](isaacs/node-tar@v6.1.11...v7.5.9) Updates `@mapbox/node-pre-gyp` from 1.0.9 to 2.0.3 - [Release notes](https://github.com/mapbox/node-pre-gyp/releases) - [Changelog](https://github.com/mapbox/node-pre-gyp/blob/master/CHANGELOG.md) - [Commits](mapbox/node-pre-gyp@v1.0.9...v2.0.3) --- updated-dependencies: - dependency-name: tar dependency-version: 7.5.9 dependency-type: indirect - dependency-name: "@mapbox/node-pre-gyp" dependency-version: 2.0.3 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com>
Upgrade actions/checkout and actions/setup-node from v1/v2 to v4 across all workflow files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace unsafe destructured regex matches with named variables and null guards to prevent runtime errors when patterns do not match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Extract ISmbProtocol interface for dependency injection and testability - SmbProtocol implements ISmbProtocol - Add withRetry() with exponential backoff for transient errors - Add SmbEndpoint interface (typed endpoint response) - Add authHeaders getter to centralise API key handling - getEndpoints now returns SmbEndpoint[] and throws on non-OK (not silent []) - deleteEndpoint now throws on non-OK (not silent false) - Remove console.log before throws; include HTTP status in error messages - broadcasterClient: throw on non-OK removeChannel response - sfuWhipResource: use ISmbProtocol instead of concrete SmbProtocol - Export ISmbProtocol from package index - Add smbProtocol.spec.ts with 7 tests covering error paths and interface conformance Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…yfills - Remove node-fetch and cross-fetch dependencies in favour of native fetch (Node 18+) - Remove custom fetch fallback function in smbProtocol.ts - Migrate whipFastifyApi.ts to async plugin signature (Fastify 5 pattern) - Replace callback-based onRequest hook with async/throw error handling - Migrate from fastify-cors to @fastify/cors with ES6 import - Update server.listen() to object options API - Upgrade fastify 3.x → 5.x and typescript 4.x → 5.x - Add 4 test spec files covering WhipFastifyApi, WrtcWhipResource, RtmpWrtcWhipResource, and RtspWrtcWhipResource (66 tests, all passing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…/server/multi-c22e25d29b' into fix/code-review-improvements
…/sdk/pbkdf2-3.1.5' into fix/code-review-improvements
…/server/multi-456de2e4f1' into fix/code-review-improvements
…/demo/base-x-3.0.11' into fix/code-review-improvements
…/sdk/sha.js-2.4.12' into fix/code-review-improvements
…/sdk/cipher-base-1.0.7' into fix/code-review-improvements
…/sdk/tmp-0.2.5' into fix/code-review-improvements
….17.23' into fix/code-review-improvements
…/server/multi-bd85718f78' into fix/code-review-improvements
…/sdk/multi-d31b51a7f2' into fix/code-review-improvements
…/server/multi-05225e0676' into fix/code-review-improvements
…/server/multi-e46b564efd' into fix/code-review-improvements
karma-typescript 5.5.4 fails to bundle CommonJS modules when TypeScript target is "esnext", leaving `exports` undefined in the browser runner (ReferenceError: exports is not defined at WHIPProtocol.js:2). Switch @eyevinn/whip-web-client tests to mocha + ts-node (same pattern as the server package). All six WHIPClient tests now run in Node.js: - TS_NODE_COMPILER_OPTIONS forces "module":"commonjs" for ts-node - spec/setup.js stubs RTCSessionDescription, RTCPeerConnection, MediaStream and MediaStreamTrack so ts-mockito can mock them and top-level spec fixtures can call `new RTCSessionDescription()` - ts-node added as devDependency (was missing, needed by mocha --require) - karma.conf.js retained but unused; test script updated 6 tests passing (was 0 — karma runner produced no results) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
typedoc@0.23.24 only supports TypeScript 4.6-4.9; upgrading to 0.28.17 which explicitly lists TypeScript 5.9.x in its peer dependencies, and updating typedoc-theme-hierarchy to 6.0.0 which requires typedoc ^0.28.0. This unblocks npm ci in CI environments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…press test logs, add job timeout
- Replace static `import { RTCPeerConnection } from "@koush/wrtc"` with a
dynamic require() inside the constructor, guarded behind the pcFactory
branch so the native binary is never loaded when a mock is injected.
- Add pcFactory parameter to WrtcWhipResource, RtspWrtcWhipResource and
RtmpWrtcWhipResource constructors; update all three test subclasses to
pass a factory instead of reassigning this.pc after super().
- Add spec/hooks.ts Mocha root hooks file that silences console.log
during tests to prevent SDP/ICE payloads from polluting terminal output.
- Wire hooks.ts into the test script via --require spec/hooks.ts.
- Add timeout-minutes: 10 to the unit-tests CI job to prevent hung runs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onnect UI - fix(server): guard optional chaining on all .at(0) SSRC/RTP lookups and skip media sections with no ssrcs to prevent runtime crashes on unexpected SDP shapes (sfuWhipResource.ts) - fix(sdk): emit 'connectionfailed' event before destroy() so callers can react to peer-connection failure without polling; drop verbose SDP log in sendOffer() - fix(demo): handle 'connectionfailed' by cleaning up card/status and showing a persistent toast with a Reconnect action that re-acquires media and creates a new WHIPClient; show video controls only while streaming; pass endpointUrl and clientOpts through ingest/createResourceCard so reconnection works correctly - fix(demo): widen toast to 480px, add close button and action-button styles for persistent actionable toasts - fix(docker-compose-sfu): set IPV4_ADDR to host machine IP (192.168.1.192) so SFU advertises reachable ICE candidates instead of loopback Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LucasMaupin
marked this pull request as draft
March 12, 2026 16:07
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
lerna bootstrap was removed in v7 and fully dropped in v9. Native npm workspaces handle dependency installation automatically. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
lerna v9 no longer handles package installation via bootstrap. Declaring workspaces lets npm ci hoist all sub-package deps (including mocha) into the root node_modules. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…lity With npm workspaces, @types/node is hoisted from the root to all packages. The previous @types/node@17 is incompatible with TS5's generic Uint8Array and causes build failures in packages/server. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
LucasMaupin
marked this pull request as ready for review
March 13, 2026 12:18
@koush/wrtc ships glibc-compiled binaries (linked against ld-linux-x86-64.so.2) that fail on Alpine Linux / musl libc environments with: Error loading shared library ld-linux-x86-64.so.2: No such file or directory Replace with @roamhq/wrtc@^0.10.0 which is an actively maintained fork with the same RTCPeerConnection API. Also update the Dockerfile to node:22-bookworm-slim (Debian/glibc) to make the glibc requirement explicit and prevent accidental base image changes to Alpine variants. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…on conflicts" This reverts commit b3aaf76.
This reverts commit ad9063e.
…mpatibility" This reverts commit e4a8cfd.
LucasMaupin
force-pushed
the
fix/code-review-improvements
branch
from
March 18, 2026 15:08
897132d to
7af1d7a
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Parcel's HTML minifier strips the redundant type="text" attribute from <input> elements during production builds (type=text is the HTML default). The CSS selector input[type="text"] only matches elements with the attribute explicitly present, so the styles were silently discarded in the production build while working fine in the Parcel dev server (which serves unminified HTML). Fix: extend all three input rules to also include input:not([type]), which matches inputs where Parcel has removed the type attribute. The checkbox input retains type=checkbox and is correctly excluded by the :not([type]) selector. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ved error handling - Add active resource cards with camera/screen icons based on capture type - Show Channel:<id> label when URL contains channelId param - Add loading resource card while connecting; remove on failure - Show player content immediately on connect attempt; hide on failure - Show toast on connection failure - Remove default WHIP endpoint URL; add .env.local support with sample - Fix status pill and resource URL vertical centering (line-height: normal) - Align footer height and idle button size to webrtc-player demo Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
birme
reviewed
Mar 19, 2026
birme
left a comment
Contributor
There was a problem hiding this comment.
To big PR to be able to make a good review but I trust that it is tested and ok.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Code review improvements addressing Fastify 5 modernisation, fetch polyfill removal, SMB resilience, comprehensive test coverage, a full demo UI redesign, SFU session reliability fixes, Fastify 5 startup-conflict resolution, SDK test suite repair, wrtc lazy-loading, null-safety hardening, lerna v9 migration to npm workspaces, and OSC deployment tooling.
Changes
Fastify 5 modernisation (
packages/server/)whipFastifyApi.tsto async plugin signature (Fastify 5 requirement)onRequesthook usesasync/throwinstead of the legacydone()callbackfastify-corsto@fastify/corsv10 (ES6 import)server.listen()to the Fastify 5 object-options APIapplication/sdpandapplication/trickle-ice-sdpfragcontent-type parsers fromwhipFastifyApiplugin — they are already registered at root level inwhipEndpoint.ts; duplicates causedFST_ERR_DUPLICATED_ROUTEon startupFST_ERR_DUPLICATED_ROUTEon startupSDP content-type parsersmoved to the root server level so all routes receive themFetch polyfill removal (
packages/server/)node-fetchandcross-fetchdependencies entirelyfetchnode-fetchimport frombroadcasterClient.tsandcross-fetchimport fromresourceManagerClient.tsundici-typesdevDependency (required for accurate@types/nodev20fetchtypings)ISmbProtocol interface + SMB resilience (
packages/server/)src/smb/ISmbProtocol.ts— interface for dependency injection and test mockingSmbProtocolnow implementsISmbProtocol;sfuWhipResource.tsuses the interface type, not the concrete classwithRetry()exponential back-off wrapper added to all SmbProtocol methods (5 attempts: 1 / 2 / 4 / 8 / 16 s)getEndpoints()anddeleteEndpoint()now throw on non-OK HTTP responses (were silently swallowing errors)SFU session reliability fixes (
packages/server/src/whip/sfu/sfuWhipResource.ts)checkChannelHealth()now starts with a 15 s initial delay — previously it was called immediately, causing sessions to be torn down before ICE could connect (ICE state is naturally'FAILED'at t=0)destroy()now wraps all SFU API calls intry/catch— DELETE no longer returns 500 when the SFU has already cleaned up the sessionforEach+asynccallback replaced withfor..offor correct error propagation.at(0)SSRC/RTP lookups; skip media sections with nossrcsto prevent runtime crashes on unexpected SDP shapeswrtc lazy-loading + test isolation (
packages/server/)import { RTCPeerConnection } from "@koush/wrtc"with a dynamicrequire()inside the constructor, guarded behind thepcFactorybranch — native binary is never loaded when a mock is injectedpcFactoryparameter added toWrtcWhipResource,RtspWrtcWhipResource, andRtmpWrtcWhipResourceconstructors; all three test subclasses updated to pass a factoryspec/hooks.tsMocha root hooks file added — silencesconsole.logduring tests to prevent SDP/ICE payloads from polluting terminal outputtimeout-minutes: 10added to theunit-testsCI jobSDK fixes (
packages/sdk/)RegExp.exec()return value inutil.ts'connectionfailed'event now emitted beforedestroy()so callers can react to peer-connection failure without pollingsendOffer()SDK test suite fix
mocha + ts-node(same pattern as the server package)karma-typescript 5.5.4fails to bundle CommonJS modules when TypeScripttargetis"esnext"— 0 tests ran in Chrome Headlessspec/setup.jsstubsRTCSessionDescription,RTCPeerConnection,MediaStream, andMediaStreamTrackso ts-mockito can mock them in Node.jsDemo UI redesign + refinements (
packages/demo/)Complete replacement of the new.css classless-framework UI with a custom dark-themed interface:
navigator.clipboardposition: absoluteprevents video from expanding the content area; video controls shown only while streamingrelbadges, Delete buttonconnectionfailedwith a Reconnect action'connectionfailed'handled by cleaning up card/status and offering a Reconnect button that re-acquires media and creates a newWHIPClientinput:not([type])selector added alongsideinput[type="text"]to survive Parcel HTML minification (strips redundanttype=textattributes in production builds)OSC deployment tooling (
packages/demo/)packages/demo/server.js— lightweight static file server for OSC/cloud deploymentsnpm run startscript added topackages/demo/package.jsonfor serving the built demopackage.jsonstartscript reverted to lerna orchestration (was temporarily changed to npm workspaces start)Lerna v9 / npm workspaces migration
lerna bootstrappostinstall script removed (dropped in lerna v7, incompatible with v9)workspacesfield added to rootpackage.jsonsonpm cihoists sub-package deps (including mocha) into rootnode_moduleslernabumped to9.0.5,fast-xml-parserbumped to4.5.4@types/nodebumped to^22.0.0at the root — required for TypeScript 5 compatibility when hoisted via workspaces (the previous@types/node@17is incompatible with TS5's genericUint8Array)Dependency updates (
packages/server/)fastify^3.27.4→^5.8.1typescript^4.6.2→^5.9.3@fastify/cors^10.1.0@types/node^20.19.37undici-typesfastify-corsnode-fetchcross-fetchCI (
packages/server/)All 4 GitHub Actions workflow files updated from
actions/*@v1/@v2to@v4.Merged dependabot PRs
The following security/maintenance PRs were merged into this branch:
brace-expansionpbkdf23.1.2 → 3.1.5serialize-javascript,mochabase-x3.0.9 → 3.0.11sha.js2.4.11 → 2.4.12cipher-base1.0.4 → 1.0.7tmp0.2.1 → 0.2.5lodash4.17.21 → 4.17.23semverqs,body-parserajv,fastifytar,@mapbox/node-pre-gyplerna8 → 9.0.5fast-xml-parserTest plan
cd packages/server && npm test— all 66 server tests passcd packages/sdk && npm test— all 6 WHIPClient tests pass (mocha + ts-node)cd packages/server && npm run build— TypeScript compiles cleanlyFST_ERR_DUPLICATED_ROUTEerrors on startupnpm ciat repo root installs cleanly via npm workspaces (mocha hoisted to rootnode_modules)cd packages/demo && npm run dev— demo loads athttp://localhost:1234with new dark UIconnectionfailedevent → persistent toast with Reconnect button appears; clicking it re-establishes the sessionnpm run buildand serve withnpm start— input styles render correctly (Parcel minification test)🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com