Update handler name to match startAndXYZ convention (#9)

Additionally, some minor cleanup and fix tests not shutting down correctly.
apollographql · Sep 19, 2022 · abbbca5 · abbbca5
1 parent 7b7a01d
commit abbbca5
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 45 deletions.
diff --git a/packages/fastify/src/index.ts b/packages/fastify/src/index.ts
@@ -27,7 +27,7 @@ export interface FastifyHandlerOptions<TContext extends BaseContext> {
 
 export function fastifyHandler(
   server: ApolloServer<BaseContext>,
-  options?: FastifyHandlerOptions<BaseContext>,
+  options?: FastifyHandlerOptions<BaseContext>
 ): RouteHandlerMethod;
 export function fastifyHandler<TContext extends BaseContext>(
   server: ApolloServer<TContext>,

diff --git a/packages/lambda/src/__tests__/integration.test.ts b/packages/lambda/src/__tests__/integration.test.ts
@@ -1,43 +1,40 @@
 import { ApolloServer, ApolloServerOptions, BaseContext } from "@apollo/server";
-import { ApolloServerPluginDrainHttpServer } from "@apollo/server/plugin/drainHttpServer";
 import {
   CreateServerForIntegrationTestsOptions,
   defineIntegrationTestSuite,
 } from "@apollo/server-integration-testsuite";
 import { createServer, Server } from "http";
 import type { AddressInfo } from "net";
 import { format } from "url";
-import { lambdaHandler } from "..";
+import { startServerAndCreateLambdaHandler } from "..";
 import { createMockServer as createAPIGatewayMockServer } from "./mockAPIGatewayServer";
 
-describe("lambdaHandler", () => {
+describe("startServerAndCreateLambdaHandler", () => {
   defineIntegrationTestSuite(
     async function (
       serverOptions: ApolloServerOptions<BaseContext>,
       testOptions?: CreateServerForIntegrationTestsOptions,
     ) {
       const httpServer = createServer();
-      const server = new ApolloServer({
-        ...serverOptions,
-        plugins: [
-          ...(serverOptions.plugins ?? []),
-          ApolloServerPluginDrainHttpServer({
-            httpServer,
-          }),
-        ],
-      });
+      const server = new ApolloServer(serverOptions);
 
       const handler = testOptions
-        ? lambdaHandler(server, testOptions)
-        : lambdaHandler(server);
+        ? startServerAndCreateLambdaHandler(server, testOptions)
+        : startServerAndCreateLambdaHandler(server);
 
       httpServer.addListener("request", createAPIGatewayMockServer(handler));
 
       await new Promise<void>((resolve) => {
         httpServer.listen({ port: 0 }, resolve);
       });
 
-      return { server, url: urlForHttpServer(httpServer) };
+      return {
+        server,
+        url: urlForHttpServer(httpServer),
+        extraCleanup: async () => {
+          httpServer.close();
+        },
+      };
     },
     {
       serverIsStartedInBackground: true,

diff --git a/packages/lambda/src/index.ts b/packages/lambda/src/index.ts
@@ -25,15 +25,18 @@ type LambdaHandler = Handler<
   APIGatewayProxyStructuredResultV2
 >;
 
-export function lambdaHandler(
+// Following the naming convention "startAndXYZ" for serverless handlers in the
+// Apollo Server docs so that it's clear the server will be started when this
+// function is called and the user should not call `start` themselves.
+export function startServerAndCreateLambdaHandler(
   server: ApolloServer<BaseContext>,
   options?: LambdaHandlerOptions<BaseContext>,
 ): LambdaHandler;
-export function lambdaHandler<TContext extends BaseContext>(
+export function startServerAndCreateLambdaHandler<TContext extends BaseContext>(
   server: ApolloServer<TContext>,
   options: WithRequired<LambdaHandlerOptions<TContext>, "context">,
 ): LambdaHandler;
-export function lambdaHandler<TContext extends BaseContext>(
+export function startServerAndCreateLambdaHandler<TContext extends BaseContext>(
   server: ApolloServer<TContext>,
   options?: LambdaHandlerOptions<TContext>,
 ): LambdaHandler {
@@ -54,33 +57,19 @@ export function lambdaHandler<TContext extends BaseContext>(
 
   return async function (event, context) {
     let parsedBody: object | string | undefined = undefined;
-    try {
-      if (!event.body) {
-        // assert there's a query string?
-      } else if (event.headers["content-type"] === "application/json") {
-        try {
-          parsedBody = JSON.parse(event.body);
-        } catch (e: unknown) {
-          return {
-            statusCode: 400,
-            body: (e as Error).message,
-          };
-        }
-      } else if (event.headers["content-type"] === "text/plain") {
-        parsedBody = event.body;
+    if (!event.body) {
+      // assert there's a query string?
+    } else if (event.headers["content-type"] === "application/json") {
+      try {
+        parsedBody = JSON.parse(event.body);
+      } catch (e: unknown) {
+        return {
+          statusCode: 400,
+          body: (e as Error).message,
+        };
       }
-    } catch (error: unknown) {
-      // The json body-parser *always* sets req.body to {} if it's unset (even
-      // if the Content-Type doesn't match), so if it isn't set, you probably
-      // forgot to set up body-parser. (Note that this may change in the future
-      // body-parser@2.)
-      // return {
-      //   statusCode: 500,
-      //   body:
-      //     '`event.body` is not set; this probably means you forgot to set up the ' +
-      //     '`body-parser` middleware before the Apollo Server middleware.',
-      // };
-      throw error;
+    } else if (event.headers["content-type"] === "text/plain") {
+      parsedBody = event.body;
     }
 
     const headers = new Map<string, string>();