Skip to content

Commit 52616e8

Browse files
authored
fix(ext/node): expose internal/js_stream_socket and add default read path (#34088)
## Summary Implements the `internal/js_stream_socket` Node compat polyfill so test fixtures and library code can `require('internal/js_stream_socket')` to wrap arbitrary `Duplex` streams in a `net.Socket` interface. The class itself already existed (used internally by `_tls_wrap.js` for TLS-over-Duplex), but two things prevented the 10 listed tests from running: 1. **It was not registered as a require()-able builtin.** Add it to the builtin module map in `ext/node/polyfills/01_require.js`. 2. **The non-TLS read path was unimplemented.** The handle's `readBuffer`/`emitEOF` were stubbed `null` and only populated by `TLSWrap.attachJsStream()`. For a bare `new JSStreamSocket(duplex)`, data read from the underlying Duplex was silently dropped and EOF never propagated, so the wrapping `Socket` never emitted `'data'` or `'end'`. Default both to forward through the standard `onread` callback installed by `net._initSocketHandle`; `TLSWrap.attachJsStream()` still overrides them when needed. Small follow-ups uncovered by the now-runnable tests: - Emit `ERR_STREAM_WRAP` (Node-style error code with the standard message) instead of a plain `Error` when the wrapped stream is in string/objectMode. - Attach `StreamWrap` as a self-reference on `JSStreamSocket` so both `const StreamWrap = require('internal/js_stream_socket')` and `const { StreamWrap } = require('internal/js_stream_socket')` patterns work — Node's module exports the class and self-references the name. - `Socket.prototype.bufferSize` returns `undefined` (not `0`) after the handle is gone, matching Node. ## Tests 8 of the 10 tests from denoland/orchid#90 now pass and are added to `tests/node_compat/config.jsonc`: - `parallel/test-stream-wrap-drain.js` - `parallel/test-stream-wrap-encoding.js` - `parallel/test-stream-wrap.js` - `parallel/test-tls-streamwrap-buffersize.js` - `parallel/test-wrap-js-stream-destroy.js` - `parallel/test-wrap-js-stream-duplex.js` - `parallel/test-wrap-js-stream-exceptions.js` - `parallel/test-wrap-js-stream-read-stop.js` Two tests still fail because of unrelated polyfill gaps and are left out of `config.jsonc`: - `parallel/test-tls-generic-stream.js` — asserts `TLSSocket._handle.writeQueueSize > 0` after a buffered write. The Rust `TLSWrap` op doesn't expose `writeQueueSize`. - `parallel/test-http-agent-domain-reused-gc.js` — relies on `handle.asyncReset(new ReusedHandle(...))` firing an `init` async hook for the `ReusedHandle` resource and a matching `before` hook on reuse. Our handles don't implement `asyncReset` and `_http_agent.js` short- circuits when the method is missing, so the hook never fires. Both are pre-existing Deno limitations beyond the scope of implementing `internal/js_stream_socket`. ## Test plan - [x] `cargo test -p node_compat_tests test-stream-wrap` — 3 pass - [x] `cargo test -p node_compat_tests test-wrap-js-stream` — 4 pass - [x] `cargo test -p node_compat_tests test-tls-streamwrap` — 1 pass - [x] Existing `test-net-buffersize.js`, `test-net-after-close.js`, `test-tls-buffersize.js`, `test-net-pingpong.js`, `test-tls-handshake-error.js`, `test-tls-server-verify.js` still pass (`bufferSize` getter change verified non-regressing) - [x] `./x fmt` clean - [x] `./x lint-js` clean Closes denoland/orchid#90
1 parent b049dfc commit 52616e8

4 files changed

Lines changed: 42 additions & 6 deletions

File tree

ext/node/polyfills/01_require.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,9 @@ const internalStreamsState =
215215
const internalSocketAddress = core.loadExtScript(
216216
"ext:deno_node/internal/socketaddress.js",
217217
);
218+
const internalJsStreamSocket = core.loadExtScript(
219+
"ext:deno_node/internal/js_stream_socket.js",
220+
).default;
218221
const internalTestBinding = core.loadExtScript(
219222
"ext:deno_node/internal/test/binding.ts",
220223
);
@@ -346,6 +349,7 @@ function setupBuiltinModules() {
346349
"internal/streams/lazy_transform": internalStreamsLazyTransform,
347350
"internal/streams/state": internalStreamsState,
348351
"internal/socketaddress": internalSocketAddress,
352+
"internal/js_stream_socket": internalJsStreamSocket,
349353
"internal/test/binding": internalTestBinding,
350354
"internal/timers": internalTimers,
351355
"internal/url": internalUrl,

ext/node/polyfills/internal/js_stream_socket.js

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,16 @@ const { codeMap, UV_ECANCELED } = core.loadExtScript(
1717
"ext:deno_node/internal_binding/uv.ts",
1818
);
1919
const {
20+
kArrayBufferOffset,
2021
kBytesWritten,
2122
kLastWriteWasAsync,
23+
kReadBytesOrError,
2224
streamBaseState,
2325
} = core.loadExtScript("ext:deno_node/internal_binding/stream_wrap.ts");
2426
const { Buffer } = core.loadExtScript("ext:deno_node/internal/buffer.mjs");
27+
const { ERR_STREAM_WRAP } = core.loadExtScript(
28+
"ext:deno_node/internal/errors.ts",
29+
);
2530

2631
const kCurrentWriteRequest = Symbol("kCurrentWriteRequest");
2732
const kCurrentShutdownRequest = Symbol("kCurrentShutdownRequest");
@@ -126,9 +131,23 @@ class JSStreamSocket extends lazyNet().Socket {
126131
shutdown(req) {
127132
return handle[kOwner].doShutdown(req);
128133
},
129-
// These are set by TLSWrap after attachJsStream()
130-
readBuffer: null,
131-
emitEOF: null,
134+
// Default read path: forward bytes from the underlying Duplex into
135+
// the wrapping Socket's readable side via the standard onread
136+
// callback (set by net._initSocketHandle).
137+
// TLSWrap.attachJsStream() overrides these to route data through
138+
// the TLS engine instead.
139+
readBuffer(chunk) {
140+
if (!handle.onread) return;
141+
const len = chunk.byteLength ?? chunk.length ?? 0;
142+
if (len === 0) return;
143+
streamBaseState[kArrayBufferOffset] = chunk.byteOffset ?? 0;
144+
streamBaseState[kReadBytesOrError] = len;
145+
handle.onread.call(handle, chunk, len);
146+
},
147+
emitEOF() {
148+
if (!handle.onread) return;
149+
handle.onread.call(handle, null, codeMap.get("EOF"));
150+
},
132151
reading: false,
133152
};
134153

@@ -142,7 +161,7 @@ class JSStreamSocket extends lazyNet().Socket {
142161
// Make sure that no further `data` events will happen.
143162
stream.pause();
144163
stream.removeListener("data", ondata);
145-
this.emit("error", new Error("Stream is not in binary mode"));
164+
this.emit("error", new ERR_STREAM_WRAP());
146165
return;
147166
}
148167

@@ -289,5 +308,11 @@ class JSStreamSocket extends lazyNet().Socket {
289308
}
290309
}
291310

311+
// Node's lib/internal/js_stream_socket exports the class as the module
312+
// itself, with `StreamWrap` as a self-reference so destructuring works:
313+
// const StreamWrap = require('internal/js_stream_socket');
314+
// const { StreamWrap } = require('internal/js_stream_socket');
315+
JSStreamSocket.StreamWrap = JSStreamSocket;
316+
292317
return { JSStreamSocket, kJSStreamHandle, kOwner, default: JSStreamSocket };
293318
})();

ext/node/polyfills/net.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,8 +1621,7 @@ Object.defineProperty(Socket.prototype, "bufferSize", {
16211621
if (this._handle) {
16221622
return this.writableLength;
16231623
}
1624-
1625-
return 0;
1624+
return undefined;
16261625
},
16271626
});
16281627

tests/node_compat/config.jsonc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3340,6 +3340,9 @@
33403340
"parallel/test-stream-unpipe-event.js": {},
33413341
"parallel/test-stream-unshift-empty-chunk.js": {},
33423342
"parallel/test-stream-unshift-read-race.js": {},
3343+
"parallel/test-stream-wrap-drain.js": {},
3344+
"parallel/test-stream-wrap-encoding.js": {},
3345+
"parallel/test-stream-wrap.js": {},
33433346
"parallel/test-stream-writable-aborted.js": {},
33443347
"parallel/test-stream-writable-change-default-encoding.js": {},
33453348
"parallel/test-stream-writable-clear-buffer.js": {},
@@ -3579,6 +3582,7 @@
35793582
"parallel/test-tls-socket-close.js": {},
35803583
"parallel/test-tls-socket-constructor-alpn-options-parsing.js": {},
35813584
"parallel/test-tls-socket-destroy.js": {},
3585+
"parallel/test-tls-streamwrap-buffersize.js": {},
35823586
"parallel/test-tls-writewrap-leak.js": {},
35833587
"parallel/test-tls-timeout-server-2.js": {},
35843588
"parallel/test-tls-tlswrap-segfault-2.js": {},
@@ -3591,6 +3595,10 @@
35913595
"parallel/test-tls-wrap-event-emmiter.js": {},
35923596
"parallel/test-tls-wrap-timeout.js": {},
35933597
"parallel/test-tls-zero-clear-in.js": {},
3598+
"parallel/test-wrap-js-stream-destroy.js": {},
3599+
"parallel/test-wrap-js-stream-duplex.js": {},
3600+
"parallel/test-wrap-js-stream-exceptions.js": {},
3601+
"parallel/test-wrap-js-stream-read-stop.js": {},
35943602
"parallel/test-trace-events-api.js": {
35953603
"ignore": true,
35963604
"reason": "Deno's node:trace_events polyfill is a stub (createTracing returns an object without .enable/.disable, getEnabledCategories returns \"\" instead of undefined). These tests were silently skipping via try/catch(require('trace_events')) until that lookup was fixed."

0 commit comments

Comments
 (0)