fix(autowatch): add guard for missing file error #69
Conversation
Added a try/catch clause to guard against a "Path doesn't exist" error coming from `MemoryFileSystem.prototype.readFileSync` when a new file is added to the file-system that matches the files glob. Unfortunately, there is not a custom type on the error, so we have to resort to string magic to determine if that's the reason for the error. Closes #40
|
I can verify that this works for me at least. Thanks for fixing it! /ping @sokra |
| @@ -156,8 +156,18 @@ Plugin.prototype.readFile = function(file, callback) { | |||
| middleware.fileSystem.readFile("/_karma_webpack_/" + file.replace(/\\/g, "/"), callback); | |||
| } | |||
| } | |||
| if(!this.waiting) | |||
| doRead(); | |||
| if (!this.waiting) { | |||
There was a problem hiding this comment.
I wonder why this fix is needed. Shouldn't be this.waiting truthy until all files are available?
There was a problem hiding this comment.
I would have assumed so, but from testing on my local machine (Windows 7 x64), this.waiting was evaluating to false whenever a new file was added that matched the glob during a watch.
I really don't understand the internals that are going on here. I simply used the logic that I would have expected to run (in the this.waiting == true branch) in the case that we observed the specific error that was getting thrown. It's entirely possible there's a simpler / more elegant way to resolve this issue. If you've got any suggestions or improvements let me know and I'd be happy to update things.
|
+1 this fixed the problem for me. |
|
@sokra: Are there any changes you'd like me to make to the PR or is it good as-is? /cc @dignifiedquire |
|
looks good to me 👍 |
|
+1 karma + webpack crash when I add a new file :(. |
|
ping @sokra |
|
@sokra: Were you looking for this to be fixed in a different manner? If you've got another path you want me to pursue I can look into that. |
|
👍 Seems like a great fix for the time being. Should be merged in. 😄 |
|
+1 |
1 similar comment
|
👍 |
|
ping @sokra |
|
Is this still a thing I need to merge @dignifiedquire? |
|
I don't think it's needed anymore. I haven't seen this error in a while anymore, so I would suggest closing it for now and if someone reproduces the issues with the latest versions looking into it again. |
|
This is still needed...
|
|
|
|
@mewdriller Evidently this is still an issue, can you please reopen a PR to the v2 branch so this issue can be properly addressed. If I don't hear anything by tomorrow evening, I will address this issue myself. |
|
Let me know if you need a repository that reproduces the issue. |
|
@Silviu-Marian - Given this was an issue under some contention in the past from what I can see above, that would be extremely helpful in getting this resolved quickly. |
Added a try/catch clause to guard against a "Path doesn't exist" error
coming from
MemoryFileSystem.prototype.readFileSyncwhen a new file isadded to the file-system that matches the files glob. Unfortunately, there
is not a custom type on the error, so we have to resort to string magic to
determine if that's the reason for the error.
Closes #40