fix: handle the side-effect artifacts case (#683)

* handle the side-effect artifacts case

* lint fix

* update comments

* update comment

* lint

* lint

* update the looksLikeArtifact logic

* update the tests

Author: joeyzhao2018

src/utils/handler.spec.ts

@@ -146,4 +146,125 @@ describe("promisifiedHandler", () => {
 
     expect(result).toEqual({ statusCode: 200, body: "Sync response" });
   });
+
+  it("waits for callback when handler returns non-promise artifact with callback parameter", async () => {
+    // Simulates aws-serverless-express pattern where a server instance is returned
+    // but the actual response comes through the callback
+    const serverArtifact = { type: "server-instance", listen: () => {} };
+    const handler: Handler = (event, context, callback) => {
+      // Simulate async processing that eventually calls callback
+      setTimeout(() => {
+        callback(null, { statusCode: 200, body: "Actual response from callback" });
+      }, 10);
+      return serverArtifact as unknown as void;
+    };
+
+    const promHandler = promisifiedHandler(handler) as any;
+
+    const result = await promHandler({}, mockContext);
+
+    // Should return the callback result, not the server artifact
+    expect(result).toEqual({ statusCode: 200, body: "Actual response from callback" });
+    expect(result).not.toBe(serverArtifact);
+  });
+
+  it("detects http.Server-like artifact (has listen AND close)", async () => {
+    // Detection method 1: Node.js http.Server or similar (has both listen and close)
+    const serverArtifact = {
+      listen: () => {},
+      close: () => {},
+      _handle: {},
+    };
+    const handler = (event: any, context: Context) => {
+      setTimeout(() => {
+        context.done(undefined, { statusCode: 200, body: "Response from context.done" });
+      }, 10);
+      return serverArtifact;
+    };
+
+    const promHandler = promisifiedHandler(handler as any) as any;
+    const result = await promHandler({}, mockContext);
+
+    expect(result).toEqual({ statusCode: 200, body: "Response from context.done" });
+    expect(result).not.toBe(serverArtifact);
+  });
+
+  it("detects EventEmitter-like artifact (has on AND emit)", async () => {
+    // Detection method 2: EventEmitter-like (has .on and .emit)
+    const emitterArtifact = {
+      on: () => {},
+      emit: () => {},
+      listeners: [],
+    };
+    const handler = (event: any, context: Context) => {
+      setTimeout(() => {
+        context.succeed({ statusCode: 200, body: "EventEmitter response" });
+      }, 10);
+      return emitterArtifact;
+    };
+
+    const promHandler = promisifiedHandler(handler as any) as any;
+    const result = await promHandler({}, mockContext);
+
+    expect(result).toEqual({ statusCode: 200, body: "EventEmitter response" });
+    expect(result).not.toBe(emitterArtifact);
+  });
+
+  it("detects EventEmitter instance", async () => {
+    // Detection method 3: Instance of EventEmitter (covers Server, Socket, etc.)
+    const { EventEmitter } = require("events");
+    const artifact = new EventEmitter();
+
+    const handler = (event: any, context: Context) => {
+      setTimeout(() => {
+        context.succeed({ statusCode: 200, body: "From EventEmitter instance" });
+      }, 10);
+      return artifact;
+    };
+
+    const promHandler = promisifiedHandler(handler as any) as any;
+    const result = await promHandler({}, mockContext);
+
+    expect(result).toEqual({ statusCode: 200, body: "From EventEmitter instance" });
+    expect(result).not.toBe(artifact);
+  });
+
+  it("detects artifact by constructor name (Server/Socket/Emitter)", async () => {
+    // Detection method 4: Constructor name matches /Server|Socket|Emitter/i
+    class CustomServer {
+      public port = 3000;
+      public start() {
+        return "started";
+      }
+    }
+    const artifact = new CustomServer();
+
+    const handler = (event: any, context: Context) => {
+      setTimeout(() => {
+        context.succeed({ statusCode: 200, body: "From CustomServer" });
+      }, 10);
+      return artifact;
+    };
+
+    const promHandler = promisifiedHandler(handler as any) as any;
+    const result = await promHandler({}, mockContext);
+
+    expect(result).toEqual({ statusCode: 200, body: "From CustomServer" });
+    expect(result).not.toBe(artifact);
+  });
+
+  it("does NOT treat plain response objects as artifacts", async () => {
+    // Plain objects that happen to have some function properties should still
+    // be treated as artifacts to be safe, but objects without functions are not artifacts
+    const handler = (event: any, context: Context) => {
+      // This is a legitimate Lambda response
+      return { statusCode: 200, body: "Plain response", headers: { "Content-Type": "text/plain" } };
+    };
+
+    const promHandler = promisifiedHandler(handler as any) as any;
+    const result = await promHandler({}, mockContext);
+
+    // Should return immediately with the response object
+    expect(result).toEqual({ statusCode: 200, body: "Plain response", headers: { "Content-Type": "text/plain" } });
+  });
 });

src/utils/handler.ts

@@ -1,5 +1,6 @@
 import { Callback, Context, Handler } from "aws-lambda";
 import { HANDLER_STREAMING, STREAM_RESPONSE } from "../constants";
+import { EventEmitter } from "events";
 
 export function promisifiedHandler<TEvent, TResult>(handler: Handler<TEvent, TResult> | any) {
   // Response Stream Lambda function.
@@ -55,18 +56,39 @@ export function promisifiedHandler<TEvent, TResult>(handler: Handler<TEvent, TRe
 
     const asyncProm = handler(event, context, modifiedCallback) as Promise<TResult> | undefined;
     let promise: Promise<TResult | undefined> = callbackProm;
-    if (asyncProm !== undefined && typeof asyncProm.then === "function") {
+
+    if (asyncProm !== undefined && typeof (asyncProm as any).then === "function") {
       // Mimics behaviour of lambda runtime, the first method of returning a result always wins.
-      promise = Promise.race([callbackProm, asyncProm]);
-    } else if (asyncProm === undefined && handler.length < 3) {
-      // Handler returned undefined and doesn't take a callback parameter, resolve immediately
-      promise = Promise.resolve(undefined);
+      promise = Promise.race([callbackProm, asyncProm as Promise<TResult>]);
     } else if (handler.length >= 3) {
       // Handler takes a callback, wait for the callback to be called
       promise = callbackProm;
     } else {
-      // Handler returned a value directly (sync handler with return value), resolve with that value
-      promise = Promise.resolve(asyncProm);
+      // Handler returned a value directly
+      // Distinguish between:
+      //  - ordinary sync return value -> resolve immediately
+      //  - side-effect artifact (e.g. aws-serverless-express server) -> wait for context.done
+
+      // Heuristic: trying to detect common types of side-effect artifacts
+      const looksLikeArtifact =
+        asyncProm !== undefined &&
+        typeof asyncProm === "object" &&
+        // 1. Node.js http.Server or similar
+        ((typeof (asyncProm as any).listen === "function" && typeof (asyncProm as any).close === "function") ||
+          // 2. EventEmitter-like (has .on and .emit)
+          (typeof (asyncProm as any).on === "function" && typeof (asyncProm as any).emit === "function") ||
+          // 3. Instance of EventEmitter (covers Server, Socket, etc.)
+          asyncProm instanceof EventEmitter ||
+          // 4. Constructor name hint
+          ((asyncProm as any).constructor && /Server|Socket|Emitter/i.test((asyncProm as any).constructor.name)));
+
+      if (looksLikeArtifact) {
+        // Wait for callbackProm instead (the context.done/succeed/fail will resolve it)
+        promise = callbackProm;
+      } else {
+        // Return the value directly
+        promise = Promise.resolve(asyncProm);
+      }
     }
     return promise;
   };