From cca753fa906b8a802a12e3b37edbffbe953c96fa Mon Sep 17 00:00:00 2001 From: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> Date: Tue, 21 Oct 2025 16:19:07 -0400 Subject: [PATCH 1/8] handle the side-effect artifacts case --- src/utils/handler.spec.ts | 46 +++++++++++++++++++++++++++++++++++++++ src/utils/handler.ts | 31 ++++++++++++++++++++------ 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/utils/handler.spec.ts b/src/utils/handler.spec.ts index 9c023960..bfd04bcc 100644 --- a/src/utils/handler.spec.ts +++ b/src/utils/handler.spec.ts @@ -146,4 +146,50 @@ 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("should wait for context.done in legacy mode when handler returns artifact (2 params)", async () => { + // This test reveals the issue: In legacy mode, handlers with only 2 parameters + // (event, context) return side-effect artifacts like aws-serverless-express server + // and rely on context.done to finish the response. The current implementation + // incorrectly resolves immediately with the artifact instead of waiting for context.done. + const serverArtifact = { type: "server-instance", listen: () => {} }; + const handler = (event: any, context: Context) => { + // Simulate legacy handler that sets up server and calls context.done + setTimeout(() => { + context.done(undefined, { statusCode: 200, body: "Response from context.done" }); + }, 10); + // Returns server artifact (side effect) but should wait for context.done + return serverArtifact; + }; + + const promHandler = promisifiedHandler(handler as any) as any; + + const result = await promHandler({}, mockContext); + + // Should wait for and return the context.done result, NOT the server artifact + // This test will FAIL because current implementation resolves immediately with serverArtifact + expect(result).toEqual({ statusCode: 200, body: "Response from context.done" }); + expect(result).not.toBe(serverArtifact); + }); }); diff --git a/src/utils/handler.ts b/src/utils/handler.ts index c60de4d9..d88c79ac 100644 --- a/src/utils/handler.ts +++ b/src/utils/handler.ts @@ -55,18 +55,35 @@ export function promisifiedHandler(handler: Handler | undefined; let promise: Promise = 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]); } 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: if returned object has at least one function-valued property, + // it's likely an artifact (like a server with `.listen`). Otherwise treat as sync result. + const looksLikeArtifact = + handler.length < 3 && + asyncProm !== undefined && + typeof asyncProm === "object" && + // check for function-valued properties + Object.keys(asyncProm as object).some((k) => typeof (asyncProm as any)[k] === "function"); + + 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; }; From 6ee8359fdbf44496e0e56c44efa05587e2565f55 Mon Sep 17 00:00:00 2001 From: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> Date: Tue, 21 Oct 2025 16:24:53 -0400 Subject: [PATCH 2/8] lint fix --- src/utils/handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/handler.ts b/src/utils/handler.ts index d88c79ac..b5688b2a 100644 --- a/src/utils/handler.ts +++ b/src/utils/handler.ts @@ -67,7 +67,7 @@ export function promisifiedHandler(handler: Handler resolve immediately // - side-effect artifact (e.g. aws-serverless-express server) -> wait for context.done - + // // Heuristic: if returned object has at least one function-valued property, // it's likely an artifact (like a server with `.listen`). Otherwise treat as sync result. const looksLikeArtifact = From 52860eacc7d3fc42acbf8a9074abb3ed4ed06050 Mon Sep 17 00:00:00 2001 From: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> Date: Tue, 21 Oct 2025 16:27:59 -0400 Subject: [PATCH 3/8] update comments --- src/utils/handler.spec.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/utils/handler.spec.ts b/src/utils/handler.spec.ts index bfd04bcc..7eb25328 100644 --- a/src/utils/handler.spec.ts +++ b/src/utils/handler.spec.ts @@ -169,10 +169,8 @@ describe("promisifiedHandler", () => { }); it("should wait for context.done in legacy mode when handler returns artifact (2 params)", async () => { - // This test reveals the issue: In legacy mode, handlers with only 2 parameters - // (event, context) return side-effect artifacts like aws-serverless-express server - // and rely on context.done to finish the response. The current implementation - // incorrectly resolves immediately with the artifact instead of waiting for context.done. + // In legacy mode, handlers with only 2 parameters (event, context) return side-effect artifacts + // like aws-serverless-express server and rely on context.done to finish the response. const serverArtifact = { type: "server-instance", listen: () => {} }; const handler = (event: any, context: Context) => { // Simulate legacy handler that sets up server and calls context.done From a69669181d64eda3bb3c5e9429865af373954a28 Mon Sep 17 00:00:00 2001 From: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> Date: Tue, 21 Oct 2025 16:31:50 -0400 Subject: [PATCH 4/8] update comment --- src/utils/handler.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils/handler.spec.ts b/src/utils/handler.spec.ts index 7eb25328..a7beedb2 100644 --- a/src/utils/handler.spec.ts +++ b/src/utils/handler.spec.ts @@ -186,7 +186,6 @@ describe("promisifiedHandler", () => { const result = await promHandler({}, mockContext); // Should wait for and return the context.done result, NOT the server artifact - // This test will FAIL because current implementation resolves immediately with serverArtifact expect(result).toEqual({ statusCode: 200, body: "Response from context.done" }); expect(result).not.toBe(serverArtifact); }); From 1aea03279f6b68fc2d7a733187fa366d85a5d286 Mon Sep 17 00:00:00 2001 From: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> Date: Tue, 21 Oct 2025 16:34:47 -0400 Subject: [PATCH 5/8] lint --- src/utils/handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/handler.ts b/src/utils/handler.ts index b5688b2a..fd34302c 100644 --- a/src/utils/handler.ts +++ b/src/utils/handler.ts @@ -67,7 +67,7 @@ export function promisifiedHandler(handler: Handler resolve immediately // - side-effect artifact (e.g. aws-serverless-express server) -> wait for context.done - // + // Heuristic: if returned object has at least one function-valued property, // it's likely an artifact (like a server with `.listen`). Otherwise treat as sync result. const looksLikeArtifact = From b637b1b024dbcf5b0fd970d355c2a8cadef4aefc Mon Sep 17 00:00:00 2001 From: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> Date: Tue, 21 Oct 2025 17:08:34 -0400 Subject: [PATCH 6/8] lint --- src/utils/handler.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/handler.spec.ts b/src/utils/handler.spec.ts index a7beedb2..9e4ba815 100644 --- a/src/utils/handler.spec.ts +++ b/src/utils/handler.spec.ts @@ -170,7 +170,7 @@ describe("promisifiedHandler", () => { it("should wait for context.done in legacy mode when handler returns artifact (2 params)", async () => { // In legacy mode, handlers with only 2 parameters (event, context) return side-effect artifacts - // like aws-serverless-express server and rely on context.done to finish the response. + // like aws-serverless-express server and rely on context.done to finish the response. const serverArtifact = { type: "server-instance", listen: () => {} }; const handler = (event: any, context: Context) => { // Simulate legacy handler that sets up server and calls context.done From de6091c35639e537dfa7bf2ffad9b3392681a84d Mon Sep 17 00:00:00 2001 From: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> Date: Wed, 22 Oct 2025 16:18:26 -0400 Subject: [PATCH 7/8] update the looksLikeArtifact logic --- src/utils/handler.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/utils/handler.ts b/src/utils/handler.ts index fd34302c..8196a913 100644 --- a/src/utils/handler.ts +++ b/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(handler: Handler | any) { // Response Stream Lambda function. @@ -68,14 +69,18 @@ export function promisifiedHandler(handler: Handler resolve immediately // - side-effect artifact (e.g. aws-serverless-express server) -> wait for context.done - // Heuristic: if returned object has at least one function-valued property, - // it's likely an artifact (like a server with `.listen`). Otherwise treat as sync result. + // Heuristic: trying to detect common types of side-effect artifacts const looksLikeArtifact = - handler.length < 3 && asyncProm !== undefined && typeof asyncProm === "object" && - // check for function-valued properties - Object.keys(asyncProm as object).some((k) => typeof (asyncProm as any)[k] === "function"); + // 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) From 6e0849c6130baf952cec81b870d557e6c8e94a76 Mon Sep 17 00:00:00 2001 From: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> Date: Wed, 22 Oct 2025 16:19:54 -0400 Subject: [PATCH 8/8] update the tests --- src/utils/handler.spec.ts | 94 +++++++++++++++++++++++++++++++++++---- 1 file changed, 86 insertions(+), 8 deletions(-) diff --git a/src/utils/handler.spec.ts b/src/utils/handler.spec.ts index 9e4ba815..e2e469a2 100644 --- a/src/utils/handler.spec.ts +++ b/src/utils/handler.spec.ts @@ -168,25 +168,103 @@ describe("promisifiedHandler", () => { expect(result).not.toBe(serverArtifact); }); - it("should wait for context.done in legacy mode when handler returns artifact (2 params)", async () => { - // In legacy mode, handlers with only 2 parameters (event, context) return side-effect artifacts - // like aws-serverless-express server and rely on context.done to finish the response. - const serverArtifact = { type: "server-instance", listen: () => {} }; + 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) => { - // Simulate legacy handler that sets up server and calls context.done setTimeout(() => { context.done(undefined, { statusCode: 200, body: "Response from context.done" }); }, 10); - // Returns server artifact (side effect) but should wait for context.done return serverArtifact; }; const promHandler = promisifiedHandler(handler as any) as any; - const result = await promHandler({}, mockContext); - // Should wait for and return the context.done result, NOT the server artifact 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" } }); + }); });