Skip to content

feat(lambda): add explicit callback completion mode - #849

Draft
joeyzhao2018 wants to merge 1 commit into
mainfrom
joey/fix-lambda-callback-completion-mode
Draft

joeyzhao2018 wants to merge 1 commit into
mainfrom
joey/fix-lambda-callback-completion-mode

Conversation

@joeyzhao2018

@joeyzhao2018 joeyzhao2018 commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds an opt-in DD_LAMBDA_HANDLER_COMPLETION=callback setting for buffered handlers that complete through a callback or a legacy context method but return an incidental synchronous value. Customers can set the Lambda environment variable without changing their handler or adding another wrapper.

For example, this handler has length === 2 because its callback parameter has a default:

exports.handler = function (event, context, callback = () => {}) {
  startWork(event, (error, response) => callback(error, response));
  return { incidental: true };
};

Today the shim selects { incidental: true } as the response. With the new setting it waits for the callback and returns the callback's response. Rest-parameter handlers (length === 0) receive the same behavior.

The default auto mode preserves existing completion selection, including ordinary synchronous responses and the server/emitter heuristics. The implementation reuses the existing callback-completion branch and reads the setting once when constructing a buffered handler. No helper signatures or tracer APIs change.

Motivation

The customer-visible problem is that the shim can select an incidental synchronous return value as the Lambda response instead of waiting for the callback's intended result. This is an existing response-selection edge case, not a regression introduced by dd-trace-js #10775.

Consider this customer handler, loaded through the normal Datadog Lambda wrapper:

exports.handler = function (event, context, callback = () => {}) {
  setTimeout(() => {
    callback(null, { result: "finished work" });
  }, 100);

  // Imagine this is an operation handle returned by a helper.
  return { operationId: "started" };
};

The customer intends { result: "finished work" } to be the invocation response. However, JavaScript reports this function's length as 2: parameters starting with the first default parameter do not count toward declared arity. A rest-parameter handler has the same ambiguity because its length is 0, even when it uses the supplied callback.

The existing shim:

  1. Calls the handler and supplies its own callback.
  2. Receives { operationId: "started" } synchronously.
  3. Sees a declared arity below three and an ordinary non-Promise return value.
  4. Selects Promise.resolve(returnValue) as completion.
  5. When the callback later provides { result: "finished work" }, it resolves a different Promise that was not selected. That callback cannot replace the selected response.

Consequently, the invocation returns:

{ "operationId": "started" }

instead of:

{ "result": "finished work" }

The invocation also stops waiting for that asynchronous work, so Lambda may freeze the environment before it finishes. This does not mean the callback always fails to run: in the AWS investigation, some callbacks ran during the wrapper's remaining cleanup, but their results still did not control the response. Selecting the incidental object happens before the runtime necessarily posts the final response.

This requires a particular combination: low declared arity, callback completion, and an ordinary non-Promise return value. A conventional (event, context, callback) handler has length three and already selects callback completion. In the current shim, an undefined return also waits for callback/context completion, and recognized server/emitter artifacts have their existing special handling. See the existing completion-selection implementation.

The shim cannot reliably infer the customer's intent here: a legitimate synchronous handler can have the same low arity and return the same object without ever calling its callback. Waiting for every supplied callback would leave those synchronous handlers pending. DD_LAMBDA_HANDLER_COMPLETION=callback makes that intent explicit while preserving existing behavior by default. A returned Promise still participates in the existing Promise/callback race; a handler that never signals completion remains pending until Lambda times out.

History and relationship to the tracer PR: this specific synchronous-return selection was introduced by datadog-lambda-js #665, merged July 7, 2025, to support genuine synchronous responses. Before that change, this particular non-undefined-return example stayed on the callback Promise. It therefore predates #10775, rather than being behavior that has existed across the library's entire history. The same wrong-result behavior reproduces with dd-trace entirely absent: the existing shim and this draft's default mode select the incidental object, while explicit callback mode waits for the intended callback result.

#10775 fixes how long the tracer's timeout monitor remains armed; this draft proposes an explicit way for the shim to choose invocation completion. They can be reviewed and released independently. Keeping this feature as a draft leaves the existing edge case unchanged and does not introduce a new customer requirement when #10775 ships. Affected handlers can already avoid the ambiguity by declaring three ordinary parameters, returning a Promise for the actual work, or not returning the incidental value.

Testing Guidelines

  • npm test -- --runInBand --watchman=false --coverage=false: 758 tests across 54 suites and 3 snapshots passed, including 30 added tests. Host Node.js was v25.8.0.
  • npm run build, npm run lint, npm run check-formatting, and git diff --check: passed.
  • SKIP_PACK=true RUNTIME_PARAM=24 CASE_PARAM=manual-callback RIE_HTTP_TRANSPORT=container ./integration_tests_local/run.sh: 9 invocations passed, including unchanged return-value and log snapshots.
  • Additional manual tests through the unmodified dist/handler.mjs entrypoint using the packed library, dd-trace 6.15.0, pinned RIE v1.36, and Lambda Node.js v24.21.0 / ARM64: 7 checks passed. Covered rest/default handlers in both modes, callback errors in callback mode, and a genuine synchronous response in auto mode. Verified one root span and its child per invocation, including error tagging. All temporary containers were removed.
  • Automated coverage includes asynchronous callback success/errors, pending callbacks, context.done/succeed/fail, Promise/callback races, synchronous throws, mode normalization/reset, synchronous responses, streaming, and the public datadog wrapper.

Additional Notes

Draft for review of the configuration name and completion contract. This is an opt-in feature; it does not attempt automatic inference of callback intent.

  • Returned Promises retain the existing first-completion-wins race with the callback in either mode.
  • In callback mode, a handler that never signals completion remains pending until Lambda times out.
  • Response-streaming handlers are unaffected. Unknown setting values fall back to auto.
  • This setting does not require a new tracer release.
  • Real AWS deployment and backend trace-ingestion verification have not been repeated for this new setting. Before release, validate unmodified rest/default handlers on Node.js 24 with the setting enabled, then unset it and verify a genuine synchronous response. The earlier AWS investigation used application adapters and is not certification of this setting.

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant