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

Got premature close error when using pump #711

Closed
Congelli501 opened this issue Jan 19, 2018 · 4 comments
Closed

Got premature close error when using pump #711

Congelli501 opened this issue Jan 19, 2018 · 4 comments

Comments

@Congelli501
Copy link
Contributor

Congelli501 commented Jan 19, 2018

If you don't know about it, pump is a module to better handle stream pipe (better error handling and better close handling to prevent resource leak) https://www.npmjs.com/package/pump.
It is considered by many as safer than pipe and may be part of node core in the future (nodejs/node#13506).

mysql2 streams are not working well with pump: a "premature close" error is thrown at the end of the stream.

Here is an example code to reproduce the error:

const mysql = require('mysql2/promise');
const pump = require('pump-promise');
const {Writable} = require('stream');

/**
 * Main test function
 */
async function main () {
	const connection = await mysql.createConnection({
		/* ... */
	});

	const sqlReadable = connection.connection.query('select 1')
		.stream();

	const testWritable = new Writable({
		objectMode: true,
		write (chunk, encoding, callback) {
			console.log(chunk);
			callback();
		}
	});

	// Not working
	await pump([
		sqlReadable,
		testWritable
	]);


	// Working
	//sqlReadable.pipe(testWritable);
	//await new Promise(resolve => testWritable.on('finish', resolve));

	connection.close();
}

main()
	.then(() => console.log('ok'))
	.catch((e) => {
		process.exitCode = 1;
		console.error(e);
	});

You can uncomment the "working" code (and comment the "not working" code) to use the working pipe version.

@sidorares
Copy link
Owner

sidorares commented Jan 19, 2018

thanks for report @Congelli501 ! Have you tried to investigate what could cause that?

In the future for promise version we'll likely prefer async iterators to stream, with api similar to something like this:

for await (const row of conn.queryRows(sql)) {
  console.log(row);
}

@Congelli501
Copy link
Contributor Author

The problem is caused by the 'close' event being emitted before the 'end' event.
Node's pipeline (https://nodejs.org/api/stream.html#stream_stream_pipeline_streams_callback) also breaks with this behaviour.

Congelli501 added a commit to Congelli501/node-mysql2 that referenced this issue Oct 13, 2018
The 'end' event (emitted by `stream.push(null)`) should always be emitted before the close event.
This fixes issue sidorares#711
sidorares added a commit that referenced this issue Oct 14, 2018
Fix issue #711: close emitted before end
@vicary
Copy link

vicary commented Oct 13, 2023

@Congelli501 @sidorares Sorry to bring this up again, but I think I hit the same issue.

Using for await still produces a premature close from the stream, I think it's caused by the row data still being transmitted.

Wrapping stream.emit('close'); inside a setImmediate would allow the data event to be consumed by the async generator before ending the stream, preventing the error.

Shall I open a new issue instead?

@nofarham
Copy link
Contributor

@sidorares any news on this issue? We're using node 18 and getting the [ERR_STREAM_PREMATURE_CLOSE]: Premature close error.
As @vicary suggested, when wrapping stream.emit('close'); inside a setImmediate it resolves the issue.

nofarham added a commit to nofarham/node-mysql2 that referenced this issue Jan 22, 2024
wellwelwel pushed a commit that referenced this issue Jan 22, 2024
* Fix node 18 (based on #711)

* Add tests to query stream

* fix async error

* fix indentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants