Repository navigation
feat(lambda): add explicit callback completion mode - #849
Draft
joeyzhao2018 wants to merge 1 commit into
Draft
joeyzhao2018 wants to merge 1 commit into
joeyzhao2018 wants to merge 1 commit into
Conversation
This was referenced Oct 20, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds an opt-in
DD_LAMBDA_HANDLER_COMPLETION=callbacksetting 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 === 2because its callback parameter has a default: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
automode 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:
The customer intends
{ result: "finished work" }to be the invocation response. However, JavaScript reports this function'slengthas 2: parameters starting with the first default parameter do not count toward declared arity. A rest-parameter handler has the same ambiguity because itslengthis 0, even when it uses the supplied callback.The existing shim:
{ operationId: "started" }synchronously.Promise.resolve(returnValue)as completion.{ 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, anundefinedreturn 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=callbackmakes 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, andgit 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.dist/handler.mjsentrypoint 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.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.
Types of Changes
Check all that apply