fix InvalidStateError exception#251
fix InvalidStateError exception#251RobbieTheWagner merged 1 commit intoember-cli-code-coverage:masterfrom brokenalarms:fix-response-text-error
InvalidStateError exception#251Conversation
|
I guess the thing I'm curious about is why is the response type |
|
It won't be. It's just to guard against the case where xhr rejects or returns falsy by expressing the only satisfactory conditions on which When this happens, and the content type is |
|
Satisfying constraints of https://xhr.spec.whatwg.org/#the-responsetext-attribute |
rwjblue
left a comment
There was a problem hiding this comment.
I definitely agree that avoiding the (IMHO bizarre) error RE: touching responseText would be better, but can you update this to add an else that throws an error RE: not being able to properly report coverage?
AFAICT this is a failure scenario, and we should error.
|
After poking at this a bit with @brokenalarms, I think we should do a few things:
|
|
Theory is there's been a non-serializable object provided sometimes in |
This commit fixes #244. The xhr.responseText property cannot be accessed unless xhr.responseType is either 'text' or '', otherwise it throws an error: Uncaught InvalidStateError: Failed to read the 'responseText' property from 'XMLHttpRequest': The value is only accessible if the object's 'responseType' is '' or 'text' (was 'json') This will therefore happen in handleCoverageResponse whenever a request returns falsy. This should never happen, but is occasionally flaking and producing the above error. This commit fixes a few likely culprits: - responseType = `json` was being set after `xhr.send`, which may cause a race condition https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/response - the middleware called res.send with a CoverageMap may receive a non-serializable object, since istanbul CoverageMap provides a toJSON method, implying that the default method may not be serializable - removing now-unused PhantomJS code. - finally, introduces a check to only read from the 'responseText' property in a valid circumstance, and gives a more meaningful error otherwise.
|
I defer to the wisdom of @rwjblue. @brokenalarms if you haven't already, can you adjust based on his feedback? |
|
Have done, thanks! |
rwjblue
left a comment
There was a problem hiding this comment.
This looks great to me, thanks for working through it @brokenalarms!
|
Thanks for the PR @brokenalarms! |
fix
InvalidStateErrorexceptionThis commit fixes #244.
The xhr.responseText property cannot be accessed unless xhr.responseType is either 'text' or '',
otherwise it throws an error:
Uncaught InvalidStateError: Failed to read the 'responseText' property from 'XMLHttpRequest':
The value is only accessible if the object's 'responseType' is '' or 'text' (was 'json')
This will therefore happen in handleCoverageResponse whenever
a request returns falsy. This should never happen, but is occasionally
flaking and producing the above error.
This commit fixes a few likely culprits:
jsonwas being set afterxhr.send, which may cause a race conditionhttps://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/response
object, since istanbul CoverageMap provides a toJSON method, implying that
the default method may not be serializable
property in a valid circumstance, and gives a more meaningful error otherwise.