resumeHook() now throws errors
#286
Conversation
`resumeHook()` now throws errors (including when a Hook is not found for a given "token") instead of returning `null`
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0e46529 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Suggestion:
The resumeWebhook function documentation and example code are outdated after the breaking change. They still suggest the function can return null when a hook is not found, but the function now throws an error instead.
View Details
📝 Patch Details
diff --git a/packages/core/src/runtime/resume-hook.ts b/packages/core/src/runtime/resume-hook.ts
index 9e05263..34df71e 100644
--- a/packages/core/src/runtime/resume-hook.ts
+++ b/packages/core/src/runtime/resume-hook.ts
@@ -139,7 +139,8 @@ export async function resumeHook<T = any>(
*
* @param token - The unique token identifying the hook
* @param request - The request to send to the hook
- * @returns Promise resolving to the response, or null if the hook doesn't exist
+ * @returns Promise resolving to the response
+ * @throws Error if the hook is not found or if there's an error during the process
*
* @example
*
@@ -155,9 +156,12 @@ export async function resumeHook<T = any>(
* return new Response('Missing token', { status: 400 });
* }
*
- * const response = await resumeWebhook(token, request);
- *
- * return response ?? new Response('Webhook not found', { status: 404 });
+ * try {
+ * const response = await resumeWebhook(token, request);
+ * return response;
+ * } catch (error) {
+ * return new Response('Webhook not found', { status: 404 });
+ * }
* }
* ```
*/
Analysis
Outdated documentation in resumeWebhook() function
What fails: The resumeWebhook() function documentation and example code suggest the function can return null when a hook is not found, but the function actually throws an error instead via getHookByToken(token).
How to reproduce: Call resumeWebhook() with a non-existent token:
const response = await resumeWebhook('nonexistent-token', request);The function will throw an error with message: "Hook with token nonexistent-token not found"
Expected behavior: According to the test suite in packages/world-local/src/storage.test.ts line 985, getByToken() throws an error for non-existent tokens rather than returning null.
Issues fixed:
- Line 141: Updated JSDoc
@returnsfrom "Promise resolving to the response, or null if the hook doesn't exist" to "Promise resolving to the response" - Line 142: Added
@throwsdocumentation - Lines 158-161: Updated example code from using null coalescing operator (
??) to use try-catch block to properly handle the thrown error
…outdated after the breaking change\. They still suggest the function can return null when a hook is not found\, but the function now throws an error instead\.
…outdated after the breaking change\. They still suggest the function can return null when a hook is not found\, but the function now throws an error instead\.
| }); | ||
| // TODO: Check for specific error types | ||
| return null; | ||
| throw err; |
There was a problem hiding this comment.
Is there a way to disambiguate if a hook is not found?
also do we need to make any docs change?
There was a problem hiding this comment.
Added some docs updates.
re: disambiguation, we need to do some follow-up work to standardize error creation in the Worlds interface. For example, world-vercel you could disambiguate by checking err.status === 404, but world-local for example returns a regular Error without status. We need to specify, document, and add to compliance tests how worlds are intended to implement this behavior.
There was a problem hiding this comment.
we tried to do this with step not found etc. (cc @adrian) so that the world would return the right error instead of a generic WorkflowAPIError or an Error
goo call standardizing the interface with a spec test
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
left comments but lgtm and can always iterate on top
There was a problem hiding this comment.
Additional Suggestions:
- The
resumemethod's return type and JSDoc still claim it can returnnull, but the underlyingresumeHook()function now throws errors instead. The type signature needs to be updated to match the new behavior.
View Details
📝 Patch Details
diff --git a/packages/core/src/define-hook.ts b/packages/core/src/define-hook.ts
index adb8adf..09ee5e5 100644
--- a/packages/core/src/define-hook.ts
+++ b/packages/core/src/define-hook.ts
@@ -60,9 +60,10 @@ export function defineHook<TInput, TOutput = TInput>({
*
* @param token - The unique token identifying the hook
* @param payload - The payload to send; if a `schema` is configured it is validated/transformed before resuming
- * @returns Promise resolving to the hook entity, or null if the hook doesn't exist
+ * @returns Promise resolving to the hook entity
+ * @throws Error if the hook is not found or if there's an error during the process
*/
- async resume(token: string, payload: TInput): Promise<HookEntity | null> {
+ async resume(token: string, payload: TInput): Promise<HookEntity> {
if (!schema?.['~standard']) {
return await resumeHook(token, payload);
}
Analysis
Type signature mismatch in defineHook().resume() return type
What fails: The resume() method in packages/core/src/define-hook.ts (line 65) has return type Promise<HookEntity | null> and JSDoc claiming it can return null, but the underlying resumeHook() function (called at lines 67 and 77) now throws errors instead of returning null.
How to reproduce: The type mismatch becomes apparent when:
- Checking commit
969ec9dwhich changedresumeHook()to throw errors instead of returningPromise<Hook | null> - Reading the JSDoc in
packages/core/src/runtime/resume-hook.tswhich now includes@throws Error if the hook is not found... - Calling
defineHook().resume()in TypeScript - the type checker allows null checks that will never execute
Result: TypeScript compiles code like:
const result = await hook.resume(...);
if (!result) { ... } // This check is now dead codeThe function throws errors instead, so null checks don't work as the type signature suggests.
Expected: Per the resumeHook() JSDoc:
- Return type should be
Promise<HookEntity>(not including null) - JSDoc should indicate it throws errors when hook not found
Fix applied: Updated packages/core/src/define-hook.ts line 63-65:
- Changed return type from
Promise<HookEntity | null>toPromise<HookEntity> - Updated JSDoc from "or null if the hook doesn't exist" to "@throws Error if the hook is not found or if there's an error during the process"
This aligns with commit 969ec9d which changed the underlying resumeHook() behavior.
View Details
📝 Patch Details
diff --git a/docs/content/docs/foundations/hooks.mdx b/docs/content/docs/foundations/hooks.mdx
index 2a8f088..3e24a73 100644
--- a/docs/content/docs/foundations/hooks.mdx
+++ b/docs/content/docs/foundations/hooks.mdx
@@ -120,10 +120,14 @@ export async function POST(request: Request) {
const slackEvent = await request.json();
const channelId = slackEvent.channel;
- // Reconstruct the token using the channel ID
- await resumeHook(`slack_messages:${channelId}`, slackEvent);
+ try {
+ // Reconstruct the token using the channel ID
+ await resumeHook(`slack_messages:${channelId}`, slackEvent);
- return new Response("OK");
+ return new Response("OK");
+ } catch (error) {
+ return new Response("Hook not found", { status: 404 });
+ }
}
```
Analysis
Unhandled error in Slack webhook example allows HTTP handler to crash
What fails: The code example in the "Custom Tokens for Deterministic Hooks" section (lines 114-128 in docs/content/docs/foundations/hooks.mdx) calls resumeHook() without error handling, but resumeHook() throws an error when the hook token is not found, causing the HTTP handler to crash with an unhandled exception.
How to reproduce:
- Use the Slack webhook handler example from the documentation
- Call the handler with a channel ID that doesn't have an associated hook
- The handler crashes with an unhandled error instead of returning an HTTP error response
Result: HTTP handler crashes with unhandled error. Caller receives no response.
Expected: Handler should catch the error from resumeHook() and return an appropriate HTTP error response (404), consistent with the first example in the same documentation file and the official resumeHook API documentation.
Root cause: resumeHook() is documented to throw errors when hooks are not found (part of the breaking change from commit 969ec9d: "resumeHook() now throws errors"). The Slack webhook example was not updated to handle this breaking change, unlike the first example in the same file.
View Details
📝 Patch Details
diff --git a/docs/content/docs/foundations/hooks.mdx b/docs/content/docs/foundations/hooks.mdx
index 2a8f088..79f215b 100644
--- a/docs/content/docs/foundations/hooks.mdx
+++ b/docs/content/docs/foundations/hooks.mdx
@@ -396,10 +396,13 @@ export async function documentApprovalWorkflow(documentId: string) {
export async function POST(request: Request) {
const { documentId, ...approvalData } = await request.json();
- // The schema validates the payload before resuming the workflow
- await approvalHook.resume(`approval:${documentId}`, approvalData);
-
- return new Response("OK");
+ try {
+ // The schema validates the payload before resuming the workflow
+ await approvalHook.resume(`approval:${documentId}`, approvalData);
+ return new Response("OK");
+ } catch (error) {
+ return Response.json({ error: "Invalid token or validation failed" }, { status: 400 });
+ }
}
```
Analysis
Missing error handling in defineHook().resume() documentation example
What fails: The "Type-Safe Hooks with defineHook()" example (lines 395-404 in docs/content/docs/foundations/hooks.mdx) calls approvalHook.resume() without error handling, but the function throws errors on validation failure or when the hook token is invalid.
How to reproduce: The example shows:
await approvalHook.resume(`approval: If approvalData fails schema validation (defined in the example with Zod), or if the token is not found, resume() throws an error.
Result: Unhandled errors crash the API route with no error response to the client.
Expected behavior: The example should wrap the call in try-catch, consistent with the earlier documentation example (lines 59-73) which correctly shows:
try {
const result = await resumeHook(token, { approved, comment });
return Response.json({ success: true, runId: result.runId });
} catch (error) {
return Response.json({ error: "Invalid token" }, { status: 404 });
}This is confirmed by:
- The
defineHook().resume()implementation inpackages/core/src/define-hook.tswhich throws on schema validation failure - The
resumeHook()JSDoc inpackages/core/src/runtime/resume-hook.tsstating@throws Error if the hook is not found - The test in
packages/core/src/define-hook.test.tsconfirming validation errors are thrown
…ut `resumeHook()` now throws errors instead of returning null\, creating a type safety mismatch\.
… `Promise<HookEntity | null>`\, but since `resumeHook()` now throws errors instead of returning null\, the return type should be `Promise<HookEntity>`\.
…ll\-check pattern\, but `resumeHook()` now throws errors instead of returning null\, making the check ineffective\.
…return `null`\, but the underlying `resumeHook()` function now throws errors instead\. The type signature needs to be updated to match the new behavior\.
…ndling\, but it now throws errors\. The example should wrap the call in try\-catch to handle errors properly\.
…but `resumeHook()` now throws errors instead of returning null\. The example should wrap the call in try\-catch to handle errors properly\.
There was a problem hiding this comment.
Additional Suggestion:
The resumeHook() call is not wrapped in error handling. Since resumeHook() now throws errors instead of returning null, this code will crash when an error occurs during the hook resumption.
View Details
📝 Patch Details
diff --git a/workbench/example/api/hook.ts b/workbench/example/api/hook.ts
index 4a28822..7eb40d6 100644
--- a/workbench/example/api/hook.ts
+++ b/workbench/example/api/hook.ts
@@ -14,11 +14,19 @@ export const POST = async (request: Request) => {
return Response.json(null, { status: 404 });
}
- await resumeHook(hook.token, {
- ...data,
- // @ts-expect-error metadata is not typed
- customData: hook.metadata?.customData,
- });
+ try {
+ await resumeHook(hook.token, {
+ ...data,
+ // @ts-expect-error metadata is not typed
+ customData: hook.metadata?.customData,
+ });
+ } catch (error) {
+ console.error('error during resumeHook', error);
+ return Response.json(
+ { error: 'Failed to resume hook' },
+ { status: 500 }
+ );
+ }
return Response.json(hook);
};
diff --git a/workbench/hono/server.ts b/workbench/hono/server.ts
index adf73ef..63ee83b 100644
--- a/workbench/hono/server.ts
+++ b/workbench/hono/server.ts
@@ -157,11 +157,19 @@ app.post('/api/hook', async ({ req }) => {
return Response.json(null, { status: 404 });
}
- await resumeHook(hook.token, {
- ...data,
- // @ts-expect-error metadata is not typed
- customData: hook.metadata?.customData,
- });
+ try {
+ await resumeHook(hook.token, {
+ ...data,
+ // @ts-expect-error metadata is not typed
+ customData: hook.metadata?.customData,
+ });
+ } catch (error) {
+ console.error('error during resumeHook', error);
+ return Response.json(
+ { error: 'Failed to resume hook' },
+ { status: 500 }
+ );
+ }
return Response.json(hook);
});
diff --git a/workbench/nitro-v2/server/api/hook.post.ts b/workbench/nitro-v2/server/api/hook.post.ts
index 52aba06..3e5aae2 100644
--- a/workbench/nitro-v2/server/api/hook.post.ts
+++ b/workbench/nitro-v2/server/api/hook.post.ts
@@ -16,11 +16,19 @@ export default defineEventHandler(async (event) => {
return Response.json(null, { status: 404 });
}
- await resumeHook(hook.token, {
- ...data,
- // @ts-expect-error metadata is not typed
- customData: hook.metadata?.customData,
- });
+ try {
+ await resumeHook(hook.token, {
+ ...data,
+ // @ts-expect-error metadata is not typed
+ customData: hook.metadata?.customData,
+ });
+ } catch (error) {
+ console.error('error during resumeHook', error);
+ return Response.json(
+ { error: 'Failed to resume hook' },
+ { status: 500 }
+ );
+ }
return Response.json(hook);
});
diff --git a/workbench/nitro-v3/routes/api/hook.post.ts b/workbench/nitro-v3/routes/api/hook.post.ts
index 6578a4a..127a5ad 100644
--- a/workbench/nitro-v3/routes/api/hook.post.ts
+++ b/workbench/nitro-v3/routes/api/hook.post.ts
@@ -14,11 +14,19 @@ export default async ({ req }: { req: Request }) => {
return Response.json(null, { status: 404 });
}
- await resumeHook(hook.token, {
- ...data,
- // @ts-expect-error metadata is not typed
- customData: hook.metadata?.customData,
- });
+ try {
+ await resumeHook(hook.token, {
+ ...data,
+ // @ts-expect-error metadata is not typed
+ customData: hook.metadata?.customData,
+ });
+ } catch (error) {
+ console.error('error during resumeHook', error);
+ return Response.json(
+ { error: 'Failed to resume hook' },
+ { status: 500 }
+ );
+ }
return Response.json(hook);
};
diff --git a/workbench/nuxt/server/api/hook.post.ts b/workbench/nuxt/server/api/hook.post.ts
index 52aba06..3e5aae2 100644
--- a/workbench/nuxt/server/api/hook.post.ts
+++ b/workbench/nuxt/server/api/hook.post.ts
@@ -16,11 +16,19 @@ export default defineEventHandler(async (event) => {
return Response.json(null, { status: 404 });
}
- await resumeHook(hook.token, {
- ...data,
- // @ts-expect-error metadata is not typed
- customData: hook.metadata?.customData,
- });
+ try {
+ await resumeHook(hook.token, {
+ ...data,
+ // @ts-expect-error metadata is not typed
+ customData: hook.metadata?.customData,
+ });
+ } catch (error) {
+ console.error('error during resumeHook', error);
+ return Response.json(
+ { error: 'Failed to resume hook' },
+ { status: 500 }
+ );
+ }
return Response.json(hook);
});
diff --git a/workbench/sveltekit/src/routes/api/hook/+server.ts b/workbench/sveltekit/src/routes/api/hook/+server.ts
index a0254e5..514ed8b 100644
--- a/workbench/sveltekit/src/routes/api/hook/+server.ts
+++ b/workbench/sveltekit/src/routes/api/hook/+server.ts
@@ -19,11 +19,19 @@ export const POST: RequestHandler = async ({
return json(null, { status: 404 });
}
- await resumeHook(hook.token, {
- ...data,
- // @ts-expect-error metadata is not typed
- customData: hook.metadata?.customData,
- });
+ try {
+ await resumeHook(hook.token, {
+ ...data,
+ // @ts-expect-error metadata is not typed
+ customData: hook.metadata?.customData,
+ });
+ } catch (error) {
+ console.error('error during resumeHook', error);
+ return json(
+ { error: 'Failed to resume hook' },
+ { status: 500 }
+ );
+ }
return json(hook);
};
Analysis
Missing error handling for resumeHook() in workbench API handlers
What fails: The workbench example API routes call resumeHook() without try-catch blocks. Since resumeHook() throws errors (breaking change in commit 969ec9d), unhandled exceptions crash the API routes with uncaught promise rejections.
How to reproduce:
- Start any workbench (e.g., example, hono, nitro-v2, nitro-v3, nuxt, sveltekit)
- Make a POST request to
/api/hookendpoint with a token that causesresumeHook()to throw (e.g., invalid hook token, workflow queueing failure) - The API route crashes with an unhandled exception instead of returning an error response
Result: API handler throws unhandled exception, logs in console:
UnhandledPromiseRejectionWarning: Error: Hook not found or workflow queue failed
Expected: Based on the resumeHook() JSDoc documentation which explicitly documents @throws Error, errors should be caught and handled gracefully, returning a 500 response with error details.
Files affected:
- workbench/example/api/hook.ts (lines 17-21)
- workbench/hono/server.ts (POST /api/hook handler)
- workbench/nitro-v2/server/api/hook.post.ts
- workbench/nitro-v3/routes/api/hook.post.ts
- workbench/nuxt/server/api/hook.post.ts
- workbench/sveltekit/src/routes/api/hook/+server.ts
All files now wrap the resumeHook() call in try-catch blocks that return a 500 error response on failure, consistent with the error handling pattern already established in these same files for getHookByToken() calls.
resumeHook()now throws errors (including when a Hook is not found for a given "token") instead of returningnull