fix InvalidStateError exception

[brokenalarms]

fix InvalidStateError exception

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.

[rwjblue]

I guess the thing I'm curious about is why is the response type text or empty?

[brokenalarms]

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 xhr.responseText property should ever be read.

When this happens, and the content type is json (as it always is in the async request done stage), this will cause the xhr.responseText to be incorrectly read rather than just return the original falsy value of xhr.response.

[brokenalarms]

Satisfying constraints of https://xhr.spec.whatwg.org/#the-responsetext-attribute
1 - this PR
2 - state is always done in this function
3 - already exists by falling back to text in the remaining valid scenarios.

[rwjblue]

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.

[rwjblue]

After poking at this a bit with @brokenalarms, I think we should do a few things:

1. Ensure that our middleware responds with the expected content type

https://github.com/kategengler/ember-cli-code-coverage/blob/d6e2262923e7caeeb383fb6ccdf7b46e227f5715/lib/attach-middleware.js#L54-L58

2. Remove phantomjs related codepaths and comments (💀)

https://github.com/kategengler/ember-cli-code-coverage/blob/81b57078d362b16dafa0197756e9ff8938151996/lib/templates/test-body-footer.html#L2-L3

https://github.com/kategengler/ember-cli-code-coverage/blob/81b57078d362b16dafa0197756e9ff8938151996/lib/templates/test-body-footer.html#L56-L59

3. Use a guard (similar to the one in this PR) to ensure we have the expected response type, and if we don't error

[brokenalarms]

Theory is there's been a non-serializable object provided sometimes in map.getCoverageSummary() since istanbul provides additional toJSON method this infers that the getCoverageSummaryMap() alone may not always be serializable.

[RobbieTheWagner]

I defer to the wisdom of @rwjblue. @brokenalarms if you haven't already, can you adjust based on his feedback?

[brokenalarms]

Have done, thanks!

[rwjblue]

This looks great to me, thanks for working through it @brokenalarms!

[RobbieTheWagner]

Thanks for the PR @brokenalarms!