Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors in renderToPipeableStream crash Node.js process #404

Open
f0x52 opened this issue Jan 4, 2025 · 2 comments · May be fixed by #405
Open

Errors in renderToPipeableStream crash Node.js process #404

f0x52 opened this issue Jan 4, 2025 · 2 comments · May be fixed by #405

Comments

@f0x52
Copy link

f0x52 commented Jan 4, 2025

Inside renderToPipeableStream, any errors thrown in renderToChunks are caught, and then used to destroy the stream

.catch((error) => {
stream.destroy(error);
});

Calling abort() will also destroy the stream with an error.

Destroying the stream emits an 'error' event on the PassThrough stream, and because there's no error handler attached to that stream, this crashes the entire process (see stacktrace below). This stream isn't exposed (only used through the pipe(writable) wrapper function), so consumers are unable to add an error handler for this themselves.

(Error caused by a reference error inside the Preact component that's being rendered)

node:events:496
      throw er; // Unhandled 'error' event
      ^

ReferenceError: aaaa is not defined
    at Object.Component (/persist/f0x/projects/preact-streaming/lib/views/index.js:17:5)
    at _renderToString (/persist/f0x/projects/preact-streaming/node_modules/preact-render-to-string/dist/stream/node/index.js:362:27)
    at renderToString (/persist/f0x/projects/preact-streaming/node_modules/preact-render-to-string/dist/stream/node/index.js:159:22)
    at renderToChunks (/persist/f0x/projects/preact-streaming/node_modules/preact-render-to-string/dist/stream/node/index.js:679:17)
    at renderToPipeableStream (/persist/f0x/projects/preact-streaming/node_modules/preact-render-to-string/dist/stream/node/index.js:766:3)
    at renderStream (/persist/f0x/projects/preact-streaming/lib/render-stream.js:35:65)
    at /persist/f0x/projects/preact-streaming/lib/index.js:17:5
    at Layer.handle [as handle_request] (/persist/f0x/projects/preact-streaming/node_modules/express/lib/router/layer.js:95:5)
    at next (/persist/f0x/projects/preact-streaming/node_modules/express/lib/router/route.js:149:13)
    at Route.dispatch (/persist/f0x/projects/preact-streaming/node_modules/express/lib/router/route.js:119:3)
Emitted 'error' event on PassThrough instance at:
    at emitErrorNT (node:internal/streams/destroy:169:8)
    at emitErrorCloseNT (node:internal/streams/destroy:128:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

Node.js v20.18.1

There could be an internal error handler attached to the stream which ensures the process doesn't crash, and could call the existing options.onError() hook (or a differently named error callback).

Or the raw stream could be returned from renderToPipeableStream so users can add the error handler themselves, or even use the stream with the more modern stream.pipeline method, which also makes it easier to handle errors properly.
The latter would also allow directly setting response.body = stream when used with the koa webserver.

@f0x52
Copy link
Author

f0x52 commented Jan 4, 2025

Minimally reproducible example

const { h } = require("preact");
const { renderToPipeableStream } = require("preact-render-to-string/stream-node");

function MyComponent() {
	const a = aaa // ReferenceError
}

try {
	const stream = renderToPipeableStream(h(MyComponent), {
		onError(e) {
			// doesn't catch the error
			console.error("caught error", e);
		},
	});
	// can't do stream.on("error") because the actual PassThrough stream isn't exposed
} catch (e) {
	// doesn't catch the error
	console.log("caught error", e);
}

@f0x52
Copy link
Author

f0x52 commented Jan 4, 2025

fwiw I checked this same code with React 19 and the ReferenceError is passed to the user-provided onError handler, after which the rendering stops / stream closes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant