Merge pull request #695 from pmcelhaney/no-response-2

a handler function should always return a response

.changeset/big-teachers-mix.md

@@ -0,0 +1,5 @@
+---
+"counterfact": patch
+---
+
+Fixed: a handler function (GET(), PUT(), etc) should always return a response, even if that response is just a status code. If the handler doesn't return, the server will response with a 500 error.

src/server/dispatcher.ts

@@ -274,6 +274,13 @@ export class Dispatcher {
       tools: new Tools({ headers }),
     });
 
+    if (response === undefined) {
+      return {
+        body: `The ${method} function did not return anything. Did you forget a return statement?`,
+        status: 500,
+      };
+    }
+
     const normalizedResponse = this.normalizeResponse(
       response,
       headers.accept ?? "*/*",

src/server/registry.ts

@@ -37,14 +37,14 @@ interface RequestDataWithBody extends RequestData {
 }
 
 interface Module {
-  DELETE?: (requestData: RequestData) => CounterfactResponse;
-  GET?: (requestData: RequestData) => CounterfactResponse;
-  HEAD?: (requestData: RequestData) => CounterfactResponse;
-  OPTIONS?: (requestData: RequestData) => CounterfactResponse;
-  PATCH?: (requestData: RequestData) => CounterfactResponse;
-  POST?: (requestData: RequestDataWithBody) => CounterfactResponse;
-  PUT?: (requestData: RequestDataWithBody) => CounterfactResponse;
-  TRACE?: (requestData: RequestData) => CounterfactResponse;
+  DELETE?: (requestData: RequestData) => CounterfactResponse | undefined;
+  GET?: (requestData: RequestData) => CounterfactResponse | undefined;
+  HEAD?: (requestData: RequestData) => CounterfactResponse | undefined;
+  OPTIONS?: (requestData: RequestData) => CounterfactResponse | undefined;
+  PATCH?: (requestData: RequestData) => CounterfactResponse | undefined;
+  POST?: (requestData: RequestDataWithBody) => CounterfactResponse | undefined;
+  PUT?: (requestData: RequestDataWithBody) => CounterfactResponse | undefined;
+  TRACE?: (requestData: RequestData) => CounterfactResponse | undefined;
 }
 
 type CounterfactResponseObject =

src/server/response-builder.ts

@@ -166,7 +166,7 @@ export function createResponseBuilder(
       },
 
       xml(this: ResponseBuilder, body: unknown) {
-        return this.match("text/xml", body);
+        return this.match("application/xml", body).match("text/xml", body);
       },
     }),
   });

src/server/types.d.ts

@@ -67,6 +67,14 @@ type HeaderFunction<Response extends OpenApiResponse> = <
   headers: Omit<Response["headers"], Header>;
 }>;
 
+type RandomFunction<Response extends OpenApiResponse> = <
+  Header extends string & keyof Response["headers"],
+>() => GenericResponseBuilder<{
+  content: {};
+  headers: Response["headers"];
+}>;
+
+
 interface ResponseBuilder {
   [status: number | `${number} ${string}`]: ResponseBuilder;
   content?: { body: unknown; type: string }[];
@@ -85,8 +93,7 @@ interface ResponseBuilder {
 type GenericResponseBuilder<
   Response extends OpenApiResponse = OpenApiResponse,
 > = [keyof Response["content"]] extends [never]
-  ? // eslint-disable-next-line @typescript-eslint/no-invalid-void-type
-    void
+  ? { }
   : OmitValueWhenNever<{
       header: [keyof Response["headers"]] extends [never]
         ? never
@@ -96,8 +103,9 @@ type GenericResponseBuilder<
       match: [keyof Response["content"]] extends [never]
         ? never
         : MatchFunction<Response>;
-      random: [keyof Response["content"]] extends [never] ? never : () => void;
+      random: [keyof Response["content"]] extends [never] ? never : RandomFunction<Response>;
       text: MaybeShortcut<"text/plain", Response>;
+      xml: MaybeShortcut<"application/xml" | "text/xml", Response>;
     }>;
 
 type ResponseBuilderFactory<

src/typescript-generator/operation-coder.js

@@ -22,7 +22,11 @@ export class OperationCoder extends Coder {
     const [firstResponse] = responses.map((response) => response.data);
 
     if (!("content" in firstResponse || "schema" in firstResponse)) {
-      return "() => { /* no response content specified in the OpenAPI document */ }";
+      return `($) => {
+        return $.response[${
+          firstStatusCode === "default" ? 200 : firstStatusCode
+        }];
+      }`;
     }
 
     return `($) => {

src/typescript-generator/operation-type-coder.js

@@ -111,6 +111,6 @@ export class OperationTypeCoder extends Coder {
 
     return `({ query, path, header, body, context, proxy }: { query: ${queryType}, path: ${pathType}, header: ${headerType}, body: ${bodyType}, context: ${contextTypeImportName}, response: ${responseType}, proxy: ${proxyType} }) => ${this.responseTypes(
       script,
-    )} | { status: 415, contentType: "text/plain", body: string } | void`;
+    )} | { status: 415, contentType: "text/plain", body: string } | { }`;
   }
 }

test/server/dispatcher.test.ts

@@ -387,8 +387,8 @@ describe("a dispatcher", () => {
     const registry = new Registry();
 
     registry.add("/a", {
-      // @ts-expect-error - not obvious how to make TypeScript happy here, and it's just a unit test
       GET({ response }) {
+        // eslint-disable-next-line total-functions/no-unsafe-readonly-mutable-assignment
         return response["200"]?.random();
       },
     });
@@ -743,4 +743,29 @@ describe("given an invalid path", () => {
       "Could not find a PUT method matching /your/left/foot/in/and/your/right/foot/out\n",
     );
   });
+
+  it("responds with a 500 error if the handler function does not return", async () => {
+    const registry = new Registry();
+
+    registry.add("/hello", {
+      GET() {
+        return undefined;
+      },
+    });
+
+    const dispatcher = new Dispatcher(registry, new ContextRegistry());
+    const response = await dispatcher.request({
+      body: "",
+      headers: {},
+      method: "GET",
+      path: "/hello",
+      query: {},
+      req: { path: "/hello" },
+    });
+
+    expect(response.status).toBe(500);
+    expect(response.body).toBe(
+      "The GET function did not return anything. Did you forget a return statement?",
+    );
+  });
 });

test/server/response-builder.test.ts

@@ -39,6 +39,10 @@ describe("a response builder", () => {
       { body: "hello", type: "text/plain" },
       { body: { hello: "world" }, type: "application/json" },
       { body: "<h1>Hello World</h1>", type: "text/html" },
+      {
+        body: "<root><hello>world</hello></root>",
+        type: "application/xml",
+      },
       {
         body: "<root><hello>world</hello></root>",
         type: "text/xml",