diff --git a/.changeset/silent-carrots-develop.md b/.changeset/silent-carrots-develop.md new file mode 100644 index 000000000..b565fbdac --- /dev/null +++ b/.changeset/silent-carrots-develop.md @@ -0,0 +1,5 @@ +--- +"inngest": patch +--- + +Better handle missing request body diff --git a/packages/inngest/src/components/InngestCommHandler.ts b/packages/inngest/src/components/InngestCommHandler.ts index e76fe3999..bd4679073 100644 --- a/packages/inngest/src/components/InngestCommHandler.ts +++ b/packages/inngest/src/components/InngestCommHandler.ts @@ -637,13 +637,30 @@ export class InngestCommHandler< const handler = async (...args: Input) => { const timer = new ServerTiming(); + /** + * Used for testing, allow setting action overrides externally when + * calling the handler. Always search the final argument. + */ + const lastArg = args[args.length - 1] as unknown; + const actionOverrides = + typeof lastArg === "object" && + lastArg !== null && + "actionOverrides" in lastArg && + typeof lastArg["actionOverrides"] === "object" && + lastArg["actionOverrides"] !== null + ? lastArg["actionOverrides"] + : {}; + /** * We purposefully `await` the handler, as it could be either sync or * async. */ - const rawActions = await timer - .wrap("handler", () => this.handler(...args)) - .catch(rethrowError("Serve handler failed to run")); + const rawActions = { + ...(await timer + .wrap("handler", () => this.handler(...args)) + .catch(rethrowError("Serve handler failed to run"))), + ...actionOverrides, + }; /** * Map over every `action` in `rawActions` and create a new `actions` @@ -701,6 +718,7 @@ export class InngestCommHandler< return ret; }, + ...actionOverrides, }; const [env, expectedServerKind] = await Promise.all([ @@ -985,10 +1003,37 @@ export class InngestCommHandler< method: string; headers: Promise>; }): Promise { + // This is when the request body is completely missing; it does not + // include an empty body. This commonly happens when the HTTP framework + // doesn't have body parsing middleware. + const isMissingBody = body === undefined; + try { let url = await actions.url("starting to handle request"); if (method === "POST") { + if (isMissingBody) { + this.log( + "error", + "Missing body when executing, possibly due to missing request body middleware" + ); + + return { + status: 500, + headers: { + "Content-Type": "application/json", + }, + body: stringify( + serializeError( + new Error( + "Missing request body when executing, possibly due to missing request body middleware" + ) + ) + ), + version: undefined, + }; + } + const validationResult = await signatureValidation; if (!validationResult.success) { return { @@ -1203,6 +1248,28 @@ export class InngestCommHandler< ]); if (inBandSyncRequested) { + if (isMissingBody) { + this.log( + "error", + "Missing body when syncing, possibly due to missing request body middleware" + ); + + return { + status: 500, + headers: { + "Content-Type": "application/json", + }, + body: stringify( + serializeError( + new Error( + "Missing request body when syncing, possibly due to missing request body middleware" + ) + ) + ), + version: undefined, + }; + } + // Validation can be successful if we're in dev mode and did not // actually validate a key. In this case, also check that we did indeed // use a particular key to validate. @@ -2070,7 +2137,8 @@ export interface ActionResponse< * The version of the execution engine that was used to run this action. * * If the action didn't use the execution engine (for example, a GET request - * as a health check), this will be `undefined`. + * as a health check) or would have but errored before reaching it, this will + * be `undefined`. * * If the version should be entirely omitted from the response (for example, * when sending preliminary headers when streaming), this will be `null`. diff --git a/packages/inngest/src/deno/fresh.ts b/packages/inngest/src/deno/fresh.ts index 81faed990..2fe8071d2 100644 --- a/packages/inngest/src/deno/fresh.ts +++ b/packages/inngest/src/deno/fresh.ts @@ -70,8 +70,8 @@ export const serve = ( const fn = handler.createHandler(); - return function handleRequest(req: Request) { - return fn(req, Deno.env.toObject()); + return function handleRequest(req: Request, ...other) { + return fn(req, Deno.env.toObject(), ...other); }; }; diff --git a/packages/inngest/src/test/helpers.ts b/packages/inngest/src/test/helpers.ts index d24f8eed4..68a140565 100644 --- a/packages/inngest/src/test/helpers.ts +++ b/packages/inngest/src/test/helpers.ts @@ -5,6 +5,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-call */ import { Inngest, InngestFunction } from "@local"; import { + HandlerResponse, InngestCommHandler, type ServeHandlerOptions, } from "@local/components/InngestCommHandler"; @@ -260,7 +261,16 @@ export const testFramework = ( const run = async ( handlerOpts: Parameters<(typeof handler)["serve"]> | ServeHandler, reqOpts: Parameters, - env: Env = {} + env: Env = {}, + + /** + * Forced action overrides, directly overwriting any fields returned by the + * handler. + * + * This is useful to produce specific scenarios where actions like body + * parsing may be missing from a framework. + */ + actionOverrides?: Partial ): Promise => { const serveHandler = Array.isArray(handlerOpts) ? getServeHandler(handlerOpts) @@ -302,6 +312,10 @@ export const testFramework = ( } const args = opts?.transformReq?.(req, res, envToPass) ?? [req, res]; + if (actionOverrides) { + args.push({ actionOverrides }); + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any const ret = await (serveHandler as (...args: any[]) => any)(...args); @@ -847,12 +861,14 @@ export const testFramework = ( requestedSyncKind, validSignature, allowInBandSync, + actionOverrides, }: { serverMode: serverKind; sdkMode: serverKind; requestedSyncKind: syncKind | undefined; validSignature: boolean | undefined; allowInBandSync: boolean; + actionOverrides?: Partial; } ) => { const signingKey = "123"; @@ -905,7 +921,8 @@ export const testFramework = ( [envKeys.InngestAllowInBandSync]: allowInBandSync ? "true" : "false", - } + }, + actionOverrides ); if (typeof expectedResponse === "number") { @@ -1033,11 +1050,47 @@ export const testFramework = ( }); }); }); + + describe("#789 with no body", () => { + Object.values(serverKind).forEach((serverMode) => { + Object.values(serverKind).forEach((sdkMode) => { + expectResponse(500, { + actionOverrides: { body: () => undefined }, + serverMode, + sdkMode, + requestedSyncKind: syncKind.InBand, + validSignature: true, + allowInBandSync: true, + }); + }); + }); + }); }); }); }); describe("POST (run function)", () => { + describe("#789 missing body", () => { + test("returns 500", async () => { + const client = createClient({ id: "test" }); + + const fn = client.createFunction( + { name: "Test", id: "test" }, + { event: "demo/event.sent" }, + () => "fn" + ); + + const ret = await run( + [{ client, functions: [fn] }], + [{ method: "POST" }], + {}, + { body: () => undefined } + ); + + expect(ret.status).toEqual(500); + }); + }); + describe("signature validation", () => { const client = createClient({ id: "test" });