Skip to content

Conversation

dedwardstech
Copy link
Contributor

@dedwardstech dedwardstech commented Jan 30, 2022

I've added documentation in the form of an example around the got.stream response event.

Fixes #1955

Checklist

  • I have read the documentation.
  • I have included a pull request description of my changes.
  • I have included some tests.
  • If it's a new feature, I have included documentation updates in both the README and the types.


const readStream = got.stream('http://example.com/image.png', {throwHttpErrors: false});

readStream.on('response', async response => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using async function for an event listener is an anti-pattern as any thrown error will become an unhandled error. Instead, wrap the async stuff in an async-IIFE and use try/catch to properly handle any error.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a missing try/catch, there will be an unhandled error regardless whether it's an async function or an async IIFE.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hence, why I'm saying there should be a try/catch here. I think only pipeline could throw here, so it should be:

(async () => {
	try {
		await pipeline(
	 		readStream,
	 		createWriteStream('image.png')
	 	);
	} catch {
		// handle it
	}
})();

Right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be one way. Another is:

readStream.on('response', async response => {
	try {
		  await pipeline(
			  readStream,
			  createWriteStream('image.png')
		  );
	} catch (error) {
		// ...
	}
});

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've modified the code. Should be fine now.

@sindresorhus sindresorhus changed the title update stream docs with response event example (#1955) Document response event in stream docs Jan 31, 2022
@sindresorhus sindresorhus merged commit 935c365 into sindresorhus:main Feb 3, 2022
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

Successfully merging this pull request may close these issues.

How to check response status code before creating a write stream
4 participants