Add wh notification decoding#273
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a webhook-focused decoder that turns raw protobuf webhook bodies into the same typed/mapped notification payloads that the WebSocket notifier emits, including transparent unwrapping of notificationBatch frames.
Changes:
- Added
decodeServerNotificationshelper for decodingapplication/x-protobufwebhook bodies into mappedServerNotification[]. - Refactored notification decoding to a shared
extractNotifications()path used by both the WebSocket notifier and the webhook decoder (including batch unwrapping + filtering non-surfaced variants). - Added test coverage for webhook decoding/batch behavior and strengthened type-level guarantees around
ServerNotification.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/js-server-sdk/tests/webhook.test.ts | Adds runtime tests for webhook decoding, payload mapping, batch unwrapping, and supported input types. |
| packages/js-server-sdk/tests/notifications.test.ts | Adds a type-level test ensuring ServerNotification['type'] matches ExpectedEvents. |
| packages/js-server-sdk/src/ws_notifier.ts | Switches WS dispatch to use shared extraction/mapping and emit batch contents in wire order. |
| packages/js-server-sdk/src/webhook.ts | Introduces decodeServerNotifications for decoding raw webhook bodies into mapped notifications. |
| packages/js-server-sdk/src/notifications.ts | Adds ServerNotification type and extractNotifications() shared decoding/mapping logic. |
| packages/js-server-sdk/src/index.ts | Exports decodeServerNotifications and ServerNotification from the package entrypoint. |
| examples/room-manager/src/index.ts | Updates protobuf content-type parsing example to return decoded notifications (supports batched webhooks). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
czerwiukk
left a comment
There was a problem hiding this comment.
assuming the emit issue got resolved - lgtm
| * ``` | ||
| * @category Notifications | ||
| */ | ||
| export const decodeServerNotifications = (data: Uint8Array | ArrayBuffer): ServerNotification[] => |
There was a problem hiding this comment.
question: shouldn't it land in ./notifications/?
There was a problem hiding this comment.
Given that websocket is in its own file, it's ok for WH to be separate (though the amount of code and abstraction is minimal)
Description
Add function that decodes notifications received by webhook.
Motivation and Context
Why is this change required? What problem does it solve? If it fixes an open
issue, please link to the issue here.
Documentation impact
Types of changes
not work as expected)