catch css file not found #2206
Merged
catch css file not found #2206
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like tests exited with 1 on Windows but passed on all other platforms...? |
|
Only |
|
Looks like it's a corrupt binary? Looking at line 1685 of the console output. |
|
Restarted the tests hopefully they pass now |
|
@DeMoorJasper looks like almost every PR has failing tests 😕 |
devongovett
requested changes
Nov 5, 2018
| try { | ||
| return await md5.file(this.name); | ||
| } catch (err) { | ||
| throw new Error(err); |
There was a problem hiding this comment.
Why do we need to catch and re-throw the error here? Isn't that the same as just not catching it in the first place?
| @@ -11,11 +11,13 @@ function md5(string, encoding = 'hex') { | |||
| md5.file = function(filename) { | |||
| return new Promise((resolve, reject) => { | |||
| fs.createReadStream(filename) | |||
| .on('error', function(error) { | |||
| reject(error); | |||
There was a problem hiding this comment.
I think you can write this like before by just passing reject directly as the callback instead of making a wrapper function
|
@devongovett good points on both of your comments. Think I got it cleaned up now. Thanks! |
devongovett
approved these changes
Nov 5, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
↪️ Pull Request
This should close #2168. The issue was that
md5.jswas trying to create a hash of a non-existent file and not catching the error. I believe I've fixed it by moving the error listener above thepipeand then adding a try/catch inRawAssetongenerateHash(). I'm definitely open to suggestions on how to improve it. This is my first PR for Parcel and I haven't had a ton of experience with Node apps 😉💻 Examples
🚨 Test instructions
✔️ PR Todo
Added/updated unit tests for this change --> might need some pointers on this if necessary
Filled out test instructions (In case there aren't any unit tests)
Included links to related issues/PRs