fix(ext/node): emit DEP0174 when util.promisify wraps a Promise-returning function

[nathanwhitbot]

Summary

util.promisify(fn) should emit a DeprecationWarning (DEP0174) the first time the wrapped function is called and the original returns a Promise — Node has flagged this footgun since 19.0.0 (lib/internal/util.js does the isPromise check inline). Deno's polyfill ran the original via ReflectApply but discarded the return value, so the warning was never raised and parallel/test-util-promisify.js failed at the first expectWarning.

Fix: capture ReflectApply and feed it through isPromise; if so, emit process.emitWarning(message, "DeprecationWarning", "DEP0174"). Matches Node's behavior verbatim.

Enables parallel/test-util-promisify.js.

Test plan

• cargo test --test node_compat -- test-util-promisify (passes)
• Full cargo test --test node_compat on Linux — only pre-existing flaky failures (test-process-threadCpuUsage-worker-threads.js, test-dgram-send-cb-quelches-error.js, test-child-process-uid-gid.js, test-net-autoselectfamily.js).

[nathanwhitbot]

Scope: First commit c001540 is the open #33650 work — same scope-creep pattern @nathanwhit flagged on #33653 / #33658 / #33660. Drop it. ext/node/polyfills/fs.ts (+36/-4) shouldn't appear in this PR's diff.

Code: 254675a4 is a direct port of lib/internal/util.js#L513-516 — isPromise(ReflectApply(...)) gate, identical message, DeprecationWarning + DEP0174. LGTM.

[nathanwhitbot]

Force-pushed 0af74863c — branch had the same stale c00154065 (fs.watch signal option from #33650) as a base. Reset to upstream/main and cherry-picked just the iter65 commit. PR is now purely the util.promisify DEP0174 change + the test enable.

[nathanwhitbot]

Self-review of `0af74863c`:

Match against Node (`lib/internal/util.js:513-516`):

if (isPromise(ReflectApply(original, this, args))) {
  process.emitWarning('Calling promisify on a function that returns a Promise is likely a mistake.',
                      'DeprecationWarning', 'DEP0174');
}

Our diff:

if (isPromise(ReflectApply(original, this, args))) {
  globalThis.process.emitWarning(
    "Calling promisify on a function that returns a Promise is likely a mistake.",
    "DeprecationWarning",
    "DEP0174",
  );
}

String, code, level, and `isPromise(ReflectApply(...))` placement all line up. ✓

Behavior preservation: previously `ReflectApply(original, this, args)` was called unconditionally; the new form still calls `ReflectApply` unconditionally (just consumes its return value). For non-Promise-returning callbacks the observable behavior is unchanged.

Per-call vs once-only emission: `process.emitWarning` dedupes by `code` internally, so passing `"DEP0174"` makes the warning fire on the first matching call across the lifetime of the process even though we call `emitWarning` on every invocation that hits the path. This is the same shape Node uses — both rely on the warning subsystem's de-dup, neither uses a per-wrapper `warned` flag.

`isPromise` source: `ext:deno_node/internal/util/types.ts`. Quick check that it returns `true` for native Promises (it uses `%IsPromise%` / `Object.prototype.toString` shape, matching Node's `internalBinding('types').isPromise`).

Test enrollment: `parallel/test-util-promisify.js` is alphabetically positioned between `test-util-promisify-custom-names.mjs` and `test-util-stripvtcontrolcharacters.js`. ✓

LGTM, holding to COMMENT until CI completes.

[fibibot]

Substance is right. Verified against Node's lib/internal/util.js:

const result = ReflectApply(original, this, args);
if (isPromise(result)) {
  process.emitWarning(
    'Calling promisify on a function that returns a Promise is likely a mistake.',
    'DeprecationWarning',
    'DEP0174',
  );
}

The PR matches verbatim, just inlined into the existing new Promise((resolve, reject) => { ... ReflectApply(...) ... }) block. Pre-PR the return value was discarded entirely.

Edge cases I checked:

• Function uses both callback and returns Promise: isPromise fires synchronously, warning emits, then the eventual resolve/reject from the callback still works because the callback was added to args before the apply. No double-resolve concern.
• Function returns undefined and uses callback (the common, correct case): isPromise(undefined) is false, no warning. ✓
• Function returns Promise but never calls callback (the bug this is warning about): warning fires, the outer Promise resolves when the inner Promise's chain unfolds elsewhere (or never, depending on the original). Same as Node — that's the whole point of the warning.

isPromise is now imported from internal/util/types.ts. Test enrollment alphabetically positioned correctly between test-util-promisify-custom-names.mjs and test-util-stripvtcontrolcharacters.js. ✓

CI

85 success, 0 failure, ~5 still in progress (mostly test node_compat debug macos-x86_64 shards 1/3, 2/3, 3/3 plus test unit_node debug macos-x86_64 plus test node_compat (2/3) debug windows-x86_64). Holding to COMMENT per the CI-must-complete rule. Substance is straightforward; happy to flip to APPROVE once the macos-x86_64 debug shards land green — they exercise this same code path and the windows shard already passed shards 1/3 and 3/3.

[nathanwhitbot]

test integration (2/2) debug macos-x86_64 fail is artifact upload (ENOTFOUND on GitHub endpoint after tests passed) — pure GH Actions infra flake. @nathanwhit re-run when convenient.

[fibibot]

Re-confirming after CI shards landed.

CI: 131 SUCCESS, 1 FAILURE (test integration (2/2) debug macos-x86_64), 1 aggregate ci status. The integration shard failure is infrastructure-only — all 318 integration tests pass, then a hashy: failed to commit hash integration_75a86709c3596933 (exit code Some(28)) post-run cache write fails. Not a test regression caused by this PR.

All node-compat-relevant shards green:

• All 36 test node_compat shards (3 × 6 platforms × debug+release) ✓
• All 12 test unit_node shards ✓
• All 24 test specs shards ✓

Substance was already verified: faithful port of Node's lib/internal/util.js DEP0174 emission — capture ReflectApply return value, isPromise check, process.emitWarning(..., "DeprecationWarning", "DEP0174"). The previously-discarded return value was the entire bug.