webhooks sdk for receiving and managing webhooks#2646
Conversation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
View Vercel preview at instant-www-js-webhooks-sdk-jsv.vercel.app. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/packages/webhooks/src/index.ts`:
- Around line 995-1014: processPayload currently collects handler promises into
results and awaits Promise.allSettled(results), which swallows rejections so
processRequest never sees failures; change processPayload (the async function
processPayload and its results array) to propagate handler errors by using
Promise.all(results) or by inspecting the settled results and throwing on any
rejection so the returned promise rejects, ensuring processRequest receives the
failure and webhook retry semantics work correctly.
- Line 477: The key cache currently uses only `kid` (keyCache: Record<string, {
alg: ImportAlgorithm; key: CryptoKey }>) which can collide across different
issuers; change the cache to be namespaced by `apiURI` (e.g., use a composite
key like `${apiURI}:${kid}` or a nested map `Record<string, Record<string,
...>>`) and update all usages (the `keyCache` declaration and every read/write
site, including the other occurrences referenced around the 874-893 block) so
keys are stored and looked up with the apiURI namespace to prevent
cross-environment key collisions.
- Line 921: The defaulting for tolerance incorrectly uses || which treats 0 as
falsy; change the assignment in index.ts so that the value from opts?.tolerance
is preserved when explicitly set (including 0) by using the nullish coalescing
operator (??) instead of || for the expression that assigns to tolerance
(referencing tolerance, opts, and defaultTolerance).
In `@client/sandbox/react-nextjs/pages/play/webhooks.tsx`:
- Around line 77-93: The component currently calls initAdmin once via useRef, so
adminDb keeps stale appId/adminToken; change this to recreate the admin client
whenever appId or adminToken (and other config/schema) change by replacing the
useRef(initAdmin(...)) pattern with a memoized or effect-driven initializer
(e.g., useMemo or useEffect) that calls initAdmin({ ...config, appId,
adminToken, schema }) with [config, appId, adminToken, schema] as dependencies,
then expose the live client (adminDb or adminClient) and update all usages
(previously adminDb.current and manager = adminDb.current.webhooks.manager) to
read from the current memoized client (e.g., adminDb or adminClient and
adminDb.webhooks.manager) so webhooks/data calls always use the up-to-date appId
and token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cf6f9edd-8a5d-4f3c-8698-0a09a6b604e0
⛔ Files ignored due to path filters (1)
client/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
client/README.mdclient/package.jsonclient/packages/admin/package.jsonclient/packages/admin/src/index.tsclient/packages/platform/package.jsonclient/packages/platform/src/api.tsclient/packages/platform/src/index.tsclient/packages/webhooks/README.mdclient/packages/webhooks/package.jsonclient/packages/webhooks/src/index.tsclient/packages/webhooks/tsconfig.dev.jsonclient/packages/webhooks/tsconfig.jsonclient/packages/webhooks/tsconfig.test.jsonclient/sandbox/react-nextjs/app/api/webhooks/route.tsclient/sandbox/react-nextjs/pages/api/webhooks-pages.tsclient/sandbox/react-nextjs/pages/play/webhooks.tsxclient/scripts/publish_packages.cljserver/src/instant/model/webhook.cljserver/src/instant/webhook_routes.clj
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/packages/webhooks/src/index.ts`:
- Around line 455-467: The signature header parsing loop fails when segments
contain extra spaces; in the loop over h.split(',') (the part variable) trim
whitespace before splitting and normalize the key/value (trim both k and v) so
the switch on k ('t', 'kid', 'v1') matches correctly; update the code around the
for (const part of h.split(',')) block to use trimmedPart = part.trim(), then
derive [k, v] from trimmedPart and trim k and v before the switch/assignments to
t, kid, and v1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6bd9e28f-75e1-406a-bdbd-35718deb8250
📒 Files selected for processing (5)
client/packages/webhooks/README.mdclient/packages/webhooks/src/__types__/typeUtils.tsclient/packages/webhooks/src/__types__/typesTests.tsclient/packages/webhooks/src/index.tsclient/sandbox/react-nextjs/pages/play/webhooks.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- client/sandbox/react-nextjs/pages/play/webhooks.tsx
nezaj
left a comment
There was a problem hiding this comment.
Awesome!
One suggestion from claude was to add a runtime test for the dispatch precedence logic (etype.action → etype.$default → $default).
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
client/packages/webhooks/README.md (2)
245-245: 💤 Low valueMinor: Inconsistent header casing in documentation.
Line 245 uses lowercase
instant-signature, while the rest of the documentation consistently uses capitalizedInstant-Signature(lines 108, 231, 75). HTTP headers are case-insensitive, so this isn't a functional issue, but for documentation consistency it would be clearer to match the casing used elsewhere.✏️ Suggested change
-Convenience wrapper around `validate`: pulls `instant-signature` and the body off a Web `Request` for you. +Convenience wrapper around `validate`: pulls `Instant-Signature` and the body off a Web `Request` for you.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/packages/webhooks/README.md` at line 245, The README line describing the convenience wrapper around `validate` uses lowercase `instant-signature`; update that header text to `Instant-Signature` to match the rest of the docs (e.g., other occurrences around `validate`) so casing is consistent across the documentation.
115-136: ⚡ Quick winUnused import could confuse readers.
Line 116 imports
Webhooksfrom@instantdb/admin, but the code that follows usesdb.webhooks, anddbis neither imported nor defined in this snippet. TheWebhooksimport is not used. This might confuse readers about what imports are actually needed for a webhook handler route.Consider either:
- Removing the import and adding a comment like
// Assumes db is initialized per the setup section above, or- Showing the minimal imports needed:
import { init } from '@instantdb/admin';followed by a briefdbinitialization, or- Assuming
dbis imported from a shared module:import { db } from './instant';📝 Example fix removing the unused import
// app/api/instant-webhook/route.ts -import { Webhooks } from '@instantdb/admin'; +// Assumes db is initialized per the admin SDK setup section above const { typedHandlers, combineHandlers } = db.webhooks.helpers();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/packages/webhooks/README.md` around lines 115 - 136, The snippet imports Webhooks but never uses it and references an undefined db; remove the unused import Webhooks and either (A) add a brief comment like "// Assumes db is initialized per the setup section above" to clarify that db is provided elsewhere, or (B) replace the unused import with the minimal real initialization/import for db (e.g., import init from the SDK and initialize db, or import { db } from your shared module) so that symbols used in the snippet (db, db.webhooks.processRequest, typedHandlers, combineHandlers) are actually defined and the example is self-consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/src/instant/model/webhook.clj`:
- Around line 277-289: The duplicate check currently calls sql/select-one with
check-webhook-duplicate-q and only filters out ignore-id after the query, which
can miss duplicates if the ignored row is returned first; update the SQL binding
used by uhsql/formatp in check-webhook-duplicate! to pass an :ignore-id param
(with appropriate pgtype metadata, e.g., {:pgtype "uuid"} or uuid[] as needed)
and add a WHERE clause in check-webhook-duplicate-q to exclude rows with id =
:ignore-id when :ignore-id is provided so sql/select-one never returns the
ignored webhook; keep existing bindings (id-attr-ids/actions/sink) and ensure
the symbol names check-webhook-duplicate-q, check-webhook-duplicate!, ignore-id,
id, sql/select-one, and uhsql/formatp are used to locate and update the code.
- Around line 383-388: The duplicate-check currently calls
get-by-app-id-and-webhook-id! which drops non-read (transactional) conns and
re-reads from the default pool, exposing stale data; change this to use a
version that honors the passed write/transactional conn (either add a new arity
or modify get-by-app-id-and-webhook-id! so it does not drop non-read conns) and
call that here (and at the other site around lines 425-433) so
check-webhook-duplicate! is built from the transaction-local row rather than a
new pooled read.
---
Nitpick comments:
In `@client/packages/webhooks/README.md`:
- Line 245: The README line describing the convenience wrapper around `validate`
uses lowercase `instant-signature`; update that header text to
`Instant-Signature` to match the rest of the docs (e.g., other occurrences
around `validate`) so casing is consistent across the documentation.
- Around line 115-136: The snippet imports Webhooks but never uses it and
references an undefined db; remove the unused import Webhooks and either (A) add
a brief comment like "// Assumes db is initialized per the setup section above"
to clarify that db is provided elsewhere, or (B) replace the unused import with
the minimal real initialization/import for db (e.g., import init from the SDK
and initialize db, or import { db } from your shared module) so that symbols
used in the snippet (db, db.webhooks.processRequest, typedHandlers,
combineHandlers) are actually defined and the example is self-consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 449367ce-8c99-4276-9854-b5b334e8eedb
📒 Files selected for processing (11)
client/packages/webhooks/README.mdclient/packages/webhooks/__tests__/src/helpers.test.tsclient/packages/webhooks/__tests__/src/processPayload.test.tsclient/packages/webhooks/__tests__/src/validate.test.tsclient/packages/webhooks/package.jsonclient/packages/webhooks/src/__types__/typeUtils.tsclient/packages/webhooks/src/__types__/typesTests.tsclient/packages/webhooks/src/index.tsclient/sandbox/react-nextjs/pages/play/webhooks.tsxserver/src/instant/model/webhook.cljserver/test/instant/model/webhook_test.clj
🚧 Files skipped from review as they are similar to previous changes (3)
- client/packages/webhooks/package.json
- client/sandbox/react-nextjs/pages/play/webhooks.tsx
- client/packages/webhooks/src/index.ts
Creates a new webhooks sdk for processing and managing webhooks.
The SDK is included in both the admin sdk and the platform sdk, but you can use it standalone if you like.
For processing webhooks, we include two main helpers:
processRequestfor modern frameworks that useRequest(I tested against nextjs app router and cloudflare), andprocessNodeRequestfor frameworks like express and the nextjs pages api (I've tested against express, raw httpRequest, koa, and nestjs).Here is what it looks like with nextjs app router:
If you want to handle the request on your own, we also include all of the necessary pieces to validate and fetch the payloads on the webhooks class.
We also include management functions so that you can create/update/delete webhooks, fetch webhook events, and retry events. We'll use those helpers when building out the cli for webhooks.
Next up is docs, and then I think we'll be ready to deploy everything before building out the CLI.
Also includes a couple of fixes for the backend.
Limitations
UseDatesparam and it's always false. We allow it with the admin sdk, but then we just ignore it--I felt it was better to make the types correct for the webhooks for now. We can add support for it when we makeuseDateObjectswork in the admin sdk (I think we'll want better client-side date parsing first, playing around with some ideas for that here https://github.com/instantdb/instant/tree/claude/fix-postgres-timestamp-dEmit)