Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions src/utils/handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" } });
});
});
36 changes: 29 additions & 7 deletions src/utils/handler.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change might need to also be checked if needed in dd-trace-js integration

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where in dd-trace-js?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That patching is safe. It's because it is yet another layer of wrapping above the patching done here in this repo.
My understanding is that this promisifiedHandler function exists in order to make it easier for the wrapping to add pre and post run handling, i.e., enabling it to use try { await the_promisifiedHandler} finally {}. And it just need to be done once. So dd-trace-js part should be good.

Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
};
Expand Down
Loading