[robobun]

Main covers this PR. PR #47559 (bc55d56) made the same one-line change in ScryptJob::then and added a test for it. The test of this PR passes on a build of bc55d56. Nothing here remains to merge. The text below is the PR as it was.

Problem

• crypto.scrypt calls its callback with (undefined, derivedKey) when the derivation works. Node passes null and documents it: "err is an exception object when key derivation fails, otherwise err is null".
• A caller that tests err === null, or runs assert.strictEqual(err, null), takes a call that worked for a failure.
• The cause is the success argument list in ScryptJob::then (src/runtime/node/node_crypto_binding.rs:1152). It starts with JSValue::UNDEFINED.

Fix

• The success argument list starts with JSValue::NULL.
• Correct because Node calls callback(null, buf) on success (lib/internal/crypto/scrypt.js:59 at v26.3.0). The failure path already passes the error alone, like Node.
• Verified: test/js/node/crypto/scrypt.test.ts (one new test, it fails on Bun 1.4.3 and on a debug build of main). Also test/js/node/test/parallel/test-crypto-scrypt.js and the crypto-scrypt case of AsyncLocalStorage-tracking.test.ts.

Background

• crypto.scrypt is a native function with no JS wrapper. ScryptJob derives the key on the work pool. Then ScryptJob::then runs on the JS thread and calls the user callback.
• ScryptJob::then is the only code that builds this argument list, so the fix has one possible place. A JS wrapper that maps undefined to null (the shape crypto.argon2 has) adds a closure per call.
• The other node:crypto callbacks already match Node. 18 more calls give the same first argument in Bun 1.4.3 and Node v26.3.0 (table in Notes).

Downsides

• A caller that compares err with undefined after crypto.scrypt now sees null. That code already fails on Node.
• No cost per call. One constant replaces another.

Notes

Reproduction (exit 1 on Bun 1.4.3, exit 0 on Node v26.3.0 and with this change):

import crypto from "node:crypto";
const wrap = start => new Promise((resolve, reject) => start((err, value) => (err === null ? resolve(value) : reject(err))));
let bad = 0;
for (const start of [
  cb => crypto.scrypt("secret", "salt", 16, cb),
  cb => crypto.scrypt("secret", "salt", 16, { N: 1024 }, cb),
]) {
  await wrap(start).catch(() => bad++);
}
process.exit(bad ? 1 : 0);

"Bun 1.4.3" in this text is the build 1.4.3-canary.1+367d939d9.

First callback argument on success, Bun 1.4.3 compared with Node v26.3.0:

call	Node	Bun 1.4.3
scrypt (no options, options, keylen 0)	null	undefined
pbkdf2, hkdf, randomBytes, randomFill (also with size 0)	null	null
generateKey (hmac, aes)	null	null
generateKeyPair (ed25519, x25519, ec, rsa)	null	null
sign, verify, diffieHellman, argon2	null	null
randomInt, generatePrime, checkPrime	undefined	undefined

Node passes undefined for the last row, so there is no single rule for every callback. Each call keeps its own constant.

crypto.argon2 reaches the user through a JS wrapper in src/js/node/crypto.ts that maps the native undefined to null. That path is correct and this change does not touch it.

The value passed as derivedKey is the same Buffer as before. The failure path (callback(err), one argument) does not change.

Suites run with the debug build:

• test/js/node/crypto/scrypt.test.ts: 3 pass. Without the src/ change the new test fails with - null / + undefined for both calls.
• test/js/node/test/parallel/test-crypto-scrypt.js: exit 0.
• test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts -t scrypt: 1 pass.

---

[auto-merge] gate passed · iteration 0 · 2 files touched

fails on main (without fix)

ASAN without fix: 1 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/crypto/scrypt.test.ts
rustup spent 8s installing the pinned toolchain (nightly-2026-09-15); it was missing or incomplete on this machine
bun test v1.4.4 (367d939d9)

test/js/node/crypto/scrypt.test.ts:
(pass) scrypt async does not leak callback/buffers when output allocation fails [993.49ms]
(pass) scryptSync reads its buffers only after every argument has been coerced [63.97ms]
 97 |   const plain = Promise.withResolvers<unknown[]>();
 98 |   const withOptions = Promise.withResolvers<unknown[]>();
 99 |   scrypt("secret", "salt", 16, (...args) => plain.resolve(args));
100 |   scrypt("secret", "salt", 16, { N: 1024 }, (...args) => withOptions.resolve(args));
101 | 
102 |   expect(await Promise.all([plain.promise, withOptions.promise])).toStrictEqual([
                                                                        ^
error: expect(received).toStrictEqual(expected)

  [
    [
-     null,
+     undefined,
      {
        "data": [
          5,
          255,
          174,
          188,
          202,
          65,
  
... (truncated)

release without fix: 1 FAILED
bun test v1.4.3-canary.1 (367d939d9)

test/js/node/crypto/scrypt.test.ts:
(pass) scrypt async does not leak callback/buffers when output allocation fails [56.46ms]
(pass) scryptSync reads its buffers only after every argument has been coerced [103.30ms]
 97 |   const plain = Promise.withResolvers<unknown[]>();
 98 |   const withOptions = Promise.withResolvers<unknown[]>();
 99 |   scrypt("secret", "salt", 16, (...args) => plain.resolve(args));
100 |   scrypt("secret", "salt", 16, { N: 1024 }, (...args) => withOptions.resolve(args));
101 | 
102 |   expect(await Promise.all([plain.promise, withOptions.promise])).toStrictEqual([
                                                                        ^
error: expect(received).toStrictEqual(expected)

  [
    [
-     null,
+     undefined,
      {
        "data": [
          5,
          255,
          174,
          188,
          202,
          65,
          119,
          10,
          244,
          37,
          212,
          186,
          155,
          78,
          123,
          205,
        ],
        "type": "Buffer",
      },
    ],
    [
-     null,
+     undefined,
      {
        "data": [
          235,
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/crypto/scrypt.test.ts
bun test v1.4.4 (367d939d9)

test/js/node/crypto/scrypt.test.ts:
(pass) scrypt async does not leak callback/buffers when output allocation fails [1006.69ms]
(pass) scryptSync reads its buffers only after every argument has been coerced [77.62ms]
(pass) scrypt passes null as the callback error when the derivation works [368.77ms]

 3 pass
 0 fail
 7 expect() calls
Ran 3 tests across 1 file. [6.43s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     83e1703874
  features     lto, baseline

23 deps, 146 codegen, 1179 objects in 5057ms

ninja: Entering directory `/workspace/bun/build/release'
[1/4] fetch lolhtml
[lolhtml] up to date
[2/4] fetch rust-argon2
[rust-argon2] up to date
[2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json
264 units: 192 lib, 16 proc-macro (host), 19 custom-build (host), 15 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib
[3/4] reconfigure
[1/1531] mkdir stamps
[2/1531] mkdir codegen
[3/1531] gen bake.{client,server,error}.js
-> bake.client.js, bake.server.js, bake.error.js
[4/1531] install /workspace/bun
bun install v1.4.3-canary.1 (367d939d9)

Checked 22 installs across 44 packages (no changes) [6.34s]
[5/1531] install /workspace/bun/packages/bun-error
bun install v1.4.3-canary.1 (367d939d9)

Checked 1 install across 2 packages (no changes) [321.00ms]
[6/1531] rustc unicode_ident 
[7/1531] rustc unicode_xid 
[8/1531] gen string-map defines_table
[9/1531] rustc heck 
[1
... (truncated)

diff hotspot

src/runtime/node/node_crypto_binding.rs |  2 +-
 test/js/node/crypto/scrypt.test.ts      | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                     reads  edits  tests
src/runtime/node/node_crypto_binding.rs      3      1      9
test/js/node/crypto/scrypt.test.ts           1      1      9

[robobun]

Status: closed. #47559 landed this fix on main (bc55d56), with a test.

How I reproduced it:

• I ran a wrapper that resolves only when err === null around crypto.scrypt("secret", "salt", 16, cb) and crypto.scrypt("secret", "salt", 16, { N: 1024 }, cb). The script is in the Notes of the PR body.
• Bun 1.4.3 (1.4.3-canary.1+367d939d9): both calls reject with undefined. Exit code 1.
• Node v26.3.0: both calls resolve. Exit code 0.
• A debug build of main at bc55d56: both calls resolve. Exit code 0.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: f40829e9-8878-4c35-89b3-68f623e7689d

📥 Commits

Reviewing files that changed from the base of the PR and between ff8ad61 and 49d440e.

📒 Files selected for processing: 2

• src/runtime/node/node_crypto_binding.rs
• test/js/node/crypto/scrypt.test.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

^{Updated 4:05 AM PT - Oct 17th, 2026}

❌ @robobun, your commit 49d440e has 1 failures in Build #127298 (All Failures):

• step failed outside runner - exit 1 on 🍎 any aarch64 - test-bun

---

🧪   To try this PR locally:

bunx bun-pr 45475

That installs a local version of the PR into your bun-45475 executable, so you can run:

bun-45475 --bun

[robobun]

Main covers this PR.

PR #47559 (bc55d56) made the same one-line change in ScryptJob::then (JSValue::UNDEFINED to JSValue::NULL, src/runtime/node/node_crypto_binding.rs) and added the test "scrypt passes null as the error of a successful derivation" to test/js/node/crypto/scrypt.test.ts.

I checked this on a debug build of bc55d56:

• The test of this PR ("scrypt passes null as the callback error when the derivation works") passes there. It makes the call with options and the call without options.
• crypto.scrypt("made-up", "salt", 8, { N: 2 }, e => console.log(String(e))) prints null, as Node v26.3.0 does. Bun 1.4.3 (1.4.3-canary.1+367d939d9) prints undefined.

This PR has nothing that main lacks. The one difference is that its test also makes the call without options, and both calls reach the same ScryptJob::then (scrypt_with_defaults calls scrypt).

I did not rebase it, because the rebase leaves only that second test call. A maintainer can close it.

[robobun]

Closing: this is covered by #47559. It landed the same one-line change in ScryptJob::then and a test for it in test/js/node/crypto/scrypt.test.ts (main has both since bc55d56). On a debug build of that commit, the reproduction from this PR exits 0 and its test passes.