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

Prevent uncaught exception in some cases #459

Merged
merged 6 commits into from
Mar 6, 2018

Conversation

wwwouter
Copy link
Contributor

If something goes wrong in the code contained by setImmediate, an uncaughtException is thrown.

The code in this pull requests wraps the code in a try/catch block and emits an error if an error is thrown.

@kevva
Copy link
Contributor

kevva commented Feb 23, 2018

Could you add a test for this?

index.js Outdated
const progressStream = new Transform({
transform(chunk, encoding, callback) {
downloaded += chunk.length;
try {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you move the contents of the try into a function and call it from the try, so we don't add more nesting.

@wwwouter
Copy link
Contributor Author

wwwouter commented Feb 26, 2018

I've put the code in a getResponse function and added a test.
This test will fail with the old code, but succeeds with the changes.

I've also added some files to work on VS Code on Windows. I can remove these if you want.

@wwwouter
Copy link
Contributor Author

Also: Travis is timing out, but this shows it's ok for at least node 4 and 8: https://travis-ci.org/sindresorhus/got/builds/346221646

@kevva
Copy link
Contributor

kevva commented Mar 1, 2018

I've also added some files to work on VS Code on Windows. I can remove these if you want.

Yes :).

@wwwouter
Copy link
Contributor Author

wwwouter commented Mar 5, 2018

Ok, removed the .vscode folder!

@sindresorhus sindresorhus changed the title try catch in setImmediate Prevent uncaught exception when in some cases Mar 6, 2018
@sindresorhus sindresorhus changed the title Prevent uncaught exception when in some cases Prevent uncaught exception in some cases Mar 6, 2018
@sindresorhus sindresorhus merged commit 43b51ba into sindresorhus:master Mar 6, 2018
@sindresorhus
Copy link
Owner

Thanks :)

@wwwouter
Copy link
Contributor Author

wwwouter commented Mar 6, 2018

You're welcome!
Could you take a quick peek at #461?
Now at least I can catch the error, but it would be even better if there wasn't an error to catch :)

szmarczak pushed a commit to szmarczak/got that referenced this pull request Mar 7, 2018
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.

3 participants