Repository navigation
Conversation
|
Updated 8:26 AM PT - Oct 21st, 2026
❌ @robobun, your commit 411f34f has 1 failures in
🧪 To try this PR locally: bunx bun-pr 35441That installs a local version of the PR into your bun-35441 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe Lambda runtime now formats console arguments with custom string-preserving handling for format specifiers, special values, and fallback inspection. It normalizes newlines and formats non- ChangesLambda console formatting
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bun-lambda/runtime.ts`:
- Line 26: Update the message normalization in the format call to treat CRLF as
a single line break before converting standalone LF characters, preserving
existing behavior for inputs containing only LF or CR.
In `@test/integration/bun-lambda/bun-lambda.test.ts`:
- Around line 149-153: Add the missing console-formatting cases to the
integration test: invoke info, debug, and trace; exercise %j substitution; log a
URL formed by concatenating parts; and include a message containing CRLF.
Preserve the existing log, error, warn, %s/%d, and separate-URL coverage in the
same test scenario.
- Around line 205-220: Update the Bun.spawn flow in this integration test to
begin draining proc.stdout, proc.stderr, and proc.exited immediately after
spawning, before awaiting responded. Store the three concurrent promises and
await responded only afterward, preserving the existing output and exit
collection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19ac9c44-b861-491a-96fa-a66b8957873a
📒 Files selected for processing (2)
packages/bun-lambda/runtime.tstest/integration/bun-lambda/bun-lambda.test.ts
|
Status: reworked and rebased on main. The PR no longer uses How to reproduce the problem Run Why the rework The earlier version sent every console call to
Now a string prints as it is, Order with #45571 #45571 has a copy of the earlier |
…h Bun.inspect
log() printed each argument with Bun.inspect and gave the pieces to the
native console. Bun.inspect quotes and escapes a string, and the console
then ran its format pass over text that was already printed. In
CloudWatch, console.log("url:", request.url) printed "url:" "https://...",
and console.log("GET", "/caf%c3%a9", 200) printed "GET" "/caf3%a9".
formatError() made the same Bun.inspect call, so the runtime's own string
diagnostics reached the Lambda error endpoints inside extra quotes.
formatArgs() now builds the message. A string prints as it is, and
Bun.inspect prints every other value, as before. A string first argument
is a format string: formatSpecifier() converts each parameter as node's
util.format does, and Bun.inspect prints an object. node:util itself is
not loaded. log() gives the console the level and one finished string.
formatError() sends a string cause as it is.
1e36aad to
f2ae540
Compare
There was a problem hiding this comment.
Beyond the inline findings, I also checked the runtime's own diagnostics that now pass through the new formatter (the exit(...) and warnOnce(...) messages in packages/bun-lambda/runtime.ts, e.g. lines 135, 149, 266-346 and 769-796): none of their first-argument strings contain %, so the specifier path cannot consume or drop their trailing args (the IAM policy object, the caught error). The %c3-style percent-encoded URL case is consumed as %c only when the URL is in the first argument, which matches util.format, and the test helper's using dir/using server/await using proc are scoped inside runLambda so they are released before any assertion runs.
Extended reasoning...
The change replaces per-argument Bun.inspect in the bun-lambda console override with a hand-rolled printf-style formatter and switches formatError's non-Error branch to pass strings through unquoted; it touches no auth, crypto or permission surface, only CloudWatch log text and the Lambda error payload. Three inline findings were confirmed (Error under %s losing its stack, the catch-to-Bun.inspect fallback, and the no-args early return), so a human should weigh those; the additional checks here on the runtime's own log call sites and the test's resource scoping turned up nothing further.
Findings marked 🟡 are optional suggestions and need no follow-up push.
… a cycle as [Circular] An Error under %s keeps its stack. A BigInt under %j throws, as node:util does.
| const constructor = Object.getOwnPropertyDescriptor(owner, "constructor")?.value; | ||
| return typeof constructor === "function" && (globalThis as any)[constructor.name] === constructor; |
There was a problem hiding this comment.
🟡 (optional) Lambda handlers that log a Buffer, URL or URLSearchParams with %s get the inspected structure in CloudWatch, where Node's runtime prints the text. hasBuiltInToString at packages/bun-lambda/runtime.ts:52 treats any constructor found on globalThis as built-in. Node's rule covers only ECMAScript intrinsics (the list at src/js/internal/util/inspect.js:203), so console.log("GET %s", url) prints the multi-line URL struct instead of the href and a Buffer prints its byte list. Fix: classify by Node's fixed intrinsic-name set rather than globalThis identity, so Buffer, URL, URLSearchParams and their subclasses go through String(value) while Error, Date and plain objects still inspect.
Why this was flagged
A handler calls console.log("fetching %s", new URL(request.url)) or console.log("body: %s", Buffer.from(text)); console.log is overridden at packages/bun-lambda/runtime.ts:148 and the %s arm at runtime.ts:87 calls hasBuiltInToString. The walk stops at URL.prototype, which owns toString, reads its constructor, and runtime.ts:52 finds globalThis.URL === URL, so it returns true and the value goes to Bun.inspect, the 14-line struct shown at test/js/web/url/url.test.ts:256, folded with \r into one record; a Buffer prints as its numeric byte list. Node's util.format and Bun's port at src/js/internal/util/inspect.js:2556-2558 test the constructor name against a fixed intrinsic set (inspect.js:203) that has no Buffer, URL or URLSearchParams, so they print String(value). The base branch inspected every argument, so output is not worse than base, but the comment at runtime.ts:30 and the PR state this is node:util's rule.
Verification: Triggered whenever a handler logs a URL, URLSearchParams or Buffer under %s. packages/bun-lambda/runtime.ts:52 decides "built-in" by (globalThis as any)[constructor.name] === constructor, so the %s arm at runtime.ts:87-88 calls Bun.inspect(value). Node's rule at src/js/internal/util/inspect.js:2556-2559 checks builtInObjects.has(descriptor.value.name), where URL/Buffer/URLSearchParams are absent.
Fixes #4826
Problem
console.log("url:", url)prints"url:" "https://…".console.log("GET", "/caf%c3%a9", 200)prints"GET" "/caf3%a9". The error endpoints geterrorMessage: "\"Function timed out\"".log()(packages/bun-lambda/runtime.ts:25) prints each argument withBun.inspect, which quotes a string, and the native console formats the pieces again.formatError()(:114) does the same.Fix
formatArgs()builds the message once: a string as it is, every other value withBun.inspect.formatError()uses the same rule.formatSpecifier()converts%s,%d,%i,%f,%jas node'sutil.formatdoes. Under%san object with a built-intoString(an Error, a Date) is inspected, so an Error keeps its stack.%jprints a cycle as[Circular].Bun.inspectprints%o,%O.test/integration/bun-lambda/bun-lambda.test.ts(4 of 6 fail on main). Same text on 16 binaries, 0.7.3 to 1.4.3.%son an Error, the catch informatArgs()), 1 left (console.log()with no arguments is a pre-existing drop outside this fix).Background
bunrunsruntime.ts. One stdout line is one CloudWatch record, solog()writes line breaks in a message as\r.node:util'sformat(the earlier version). It printsHeaders {}, logs the event body with aRequest, and loads 17 M instructions at cold start or in the first format-string log.log()and drops it with its bun-lambda test. These tests pass on its native change.Downsides
%j, or atoStringthat throws under%s, now throws from the console call, asutil.formatdoes on node's runtime. On main the value printed asBun.inspecttext. In a handler the throw becomes a 500 response and an ERROR record.Notes
Output (the real
runtime.tsagainst a mock Lambda Runtime API, bun 1.4.3.⏎is\r.)console.log("url:", request.url)"url:" "http://api.example.com/hello"url: http://api.example.com/helloconsole.error("payload:", JSON.stringify({ a: 1 }, null, 2))"payload:" "{\n \"a\": 1\n}"payload: {⏎ "a": 1⏎}console.log("hi %s, you are %d", "bob", 42)"hi "bob", you are 42"hi bob, you are 42console.log("GET", "/caf%c3%a9", 200)"GET" "/caf3%a9"GET /caf%c3%a9 200console.log({ url: "/caf%c3%a9" }, 200){⏎ url: "/caf3%a9",⏎}{⏎ url: "/caf%c3%a9",⏎} 200console.log("headers:", new Headers({ a: "1" }))"headers:" Headers {⏎ "a": "1",⏎}headers: Headers {⏎ "a": "1",⏎}console.log("req %s took %dms", "GET /", 12, headers)"req "GET /" took 12ms" Headers {…}req GET / took 12ms Headers {⏎ "a": "1",⏎}console.log("Event: %j", { c: [1] })"Event: "{\r c: [ 1 ],\r}""Event: {"c":[1]}ERROR "Runtime failed to send request to Lambda [status: 500]"ERROR Runtime failed to send request to Lambda [status: 500]errorMessageafter a timeout"\"Function timed out\"""Function timed out"Bun.inspecttext inerrorMessage.errorTypestays"Error".The format string
nullandundefined, every argument list prints whatutil.formatprints (node v26.3.0 and Bun's port), and%jwith a BigInt throws a TypeError as it does there.%d,%i,%fand%jconvert asutil.formatdoes.%sfollows node'shasBuiltInToStringrule: the owner oftoStringorSymbol.toPrimitivein the prototype chain is found, and when its constructor is a global built-in the value prints withBun.inspect(an Error with its stack, a Date as ISO text,{ a: 1 }). An object with its owntoStringorSymbol.toPrimitiveprintsString(value). Node usesdepth: 0for the inspect under%s. This PR uses the default depth, the same as%o.%o,%Oand the arguments after the last specifier print withBun.inspect.%jon a cycle prints[Circular]. The cyclic TypeError is told apart by its message, computed once from a known cycle, asutil.formatdoes. Every other throw (a BigInt under%j, a usertoStringthat throws) reaches the caller. A throw in a handler'sfetchis a 500 response and an ERROR record with the stack.Why not
node:utilThe earlier version of this PR sent every call to
util.format, and #45571 has a copy of thatlog(). It is what node's Lambda runtime does (aws-lambda-nodejs-runtime-interface-client,src/LogPatch.js:util.format(message, ...params), then\nto\r). In Bun it changes the printer of every value, and a Lambda handler logs web objects:console.log("headers:", request.headers)printsheaders: Headers {}. A Response, a Blob and a FormData print empty too.console.log(request)printsRequest { aws: { ..., body: '...' } }. The runtime stores the event inrequest.aws, andutil.inspectlists own properties. So the request body lands in CloudWatch. main and this PR printRequest (N bytes) { method, url, headers }.Action: [Array], Resource: [Array], becauseutil.inspectstops at depth 2.node:utilimport loads it in every execution environment: 13.16 M to 30.33 M instructions to the firstGET /invocation/next, 13.9 to 22.4 ms, 2,301 to 5,042 heap objects, for a handler that never logs.util.inspectprint the web classes. It is open. It does not change the depth or the load.A second version kept
Bun.inspectfor values and loadednode:utilonly at the first format string, with each object in a wrapper that sentnode:utilback toBun.inspect. Then the load ran inside the first invocation that logs a format string: 17.96 M instructions and 2,698 heap objects (417 KB), where main runs none. The switch informatSpecifier()runs 0.2 M there and loads nothing.The hand-off to the console
log()keeps main's call shape: the level, the request id, then one finished message where main spread the pieces. The message is the last argument, so every release prints it as it is.write(2)calls per line on bun 1.1.22 and older (the text, then the newline). This shape is one call on all 10 binaries measured, 0.7.3 to 1.4.3, as on main. It also does not copy a long message: a joined string cost 300 more instructions than main for a logged Error.\r. node's runtime writes\r\r. On main a raw CRLF does not reach the fold, becauseBun.inspectescapes it.Releases
The layer runs on the release that the user picks (
build-layer --release). 54 handler calls ran on 16 binaries: 0.7.3, 0.8.0, 0.8.1, 1.0.0, 1.0.4, 1.1.0, 1.1.22, 1.1.23, 1.1.45, 1.2.0, 1.2.23, 1.3.0, 1.3.14, 1.4.0, 1.4.3 and a 1.4.3 canary. 50 calls print the same text on all 16. The other 4 differ only byBun.inspect's own text in that release: the field order of a Response, the text of an Error, the label of a null-prototype object, and a string under%oon 0.7.3.Measurements (bun 1.4.3 release, linux x64 baseline build, transpiler cache off, the test layout with aws4fetch on disk. Instructions are
instructions:ufromperf_event_openover the process tree.)GET /invocation/next, handler never logs, median of 40 interleaved runs: main 13,158,521, this PR +502,958 (+3.8%), a second copy of main +12,094. Wall time 12.65 and 12.89 ms (second copy of main 13.50). Peak RSS 19,824 and 19,880 KB.log("request received")8,721 to 5,560.log("url:", url)12,071 to 5,685.log(obj)12,715 to 12,497.log("obj:", obj)15,198 to 12,806.log(headers)55,660 to 55,367.log(request)81,787 to 81,375.error(new Error)45,244 to 44,977.("hi %s, you are %d", "bob", 42)13,668 to 7,537.("req %s took %dms", "GET /", 12, headers)64,938 to 59,391.("Event: %j", event)45,757 to 8,905. Two copies of main differ by up to 295.heapStats()deltas, 3 runs): fewer than main on 15 of 15 shapes. For examplelog(obj)9 to 6,log(headers)14 to 11,("hi %s, you are %d", "bob", 42)12 to 8.Bun.inspectcalls perconsole.log("url:", url): 0 (main 2). Regex passes per call: 1 (main 1 per argument). Console arguments per call: 3 (main 2 plus 1 per argument).write(2)calls per log line: 1 (main 1).runtime.tsis 24,584 to 26,850 bytes.Cold start in a real layer
node_modules, as the test has. The layer zip holds onlybun,bootstrapandruntime.ts(scripts/build-layer.ts), andruntime.ts:1imports aws4fetch. So a real cold start asks the npm registry for aws4fetch before the first Runtime API call. With a registry that does not answer, the process exits witherror: Cannot find package 'aws4fetch'(Bun layer for AWS Lambda: "error: Cannot find package 'aws4fetch' from '/opt/runtime.ts'" #19756). That cost is larger than anything in this PR.fetchAws()is a separate change. It is not in this PR. Related to Bun layer for AWS Lambda: "error: Cannot find package 'aws4fetch' from '/opt/runtime.ts'" #19756 and Lambda not invoked when in VPC #6825.Tests
console prints a string as it is and every other value as Bun.inspect does: 29 console calls in one handler, at init and in an invocation, through the six methods. The expected text of each value isBun.inspectof the same value in the test process. TheRequestrow compares withBun.inspect(request)from the handler. The last assertion reads a heap marker: the runtime has not loadednode:utilafter all the calls.errorMessage of ... is the cause as console prints it: a string at init, an object at init, andFunction timed outin an invocation. The test checks the error body, stdout and the exit code.a BigInt under %j throws in the handler, as any other throw in fetch: the ERROR record holds the TypeError with its stack and the invocation posts a 500 response.runtime.ts, 4 of 6 fail. With the earlierutil.formatversion, 2 of 5 fail. With console: only a primitive-string first argument is a format string #45571's copy oflog(), 4 of 5 fail.includes("%")check that skips the specifier pass, one gives the console a single joined string.packages/, so a releasedbunruns it too. The tests ran on the debug build and on 1.4.3.Landing order with #45571
log()passes the level first and depended on the old rule, so console: only a primitive-string first argument is a format string #45571 copied this PR's earlierlog()and its console test.log(), a handler'sconsole.log("hi %s, you are %d", "bob", 42)prints"hi %s, you are %d" "bob" 42. So console: only a primitive-string first argument is a format string #45571 can drop its copy only after thislog()is on main.log()does not depend on either native rule. With console: only a primitive-string first argument is a format string #45571's native change cherry-picked on top, the 5 tests pass on the debug build.packages/bun-lambda/runtime.tshunk, its bun-lambda test and itsFixes #4826line. The two PRs conflict in those two files.Not changed
LEVEL messagebefore the first invocation,LEVEL RequestId: ID messagein one.console.dir,table,assert,group,countandtimeare not wrapped.errorTypeis"Error"for a cause that is not an Error. node's runtime sendstypeof.\nin it becomes\rinside the same record.[review] gate passed · iteration 0 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file