marvinhagemeister / xhr-mocklet

Mock XMLHttpRequests in both the browser and node
5 stars 1 forks source link

Fixes timeout behavior when MockXMLHttpRequest has a timeout #21

Closed hearnden closed 6 years ago

hearnden commented 6 years ago

Currently, the presence of a timeout on a request, forces MockXMLHttpRequest.send() to take the timeout path, even when the response returned by the handler indicates no timeout. Additionally, if the response indicates a short timeout, a long timeout on the request is preferred.

Basically, this means it's not possible to test the non-timeout path of code that sends a request with a timeout. i.e., code like this:

const req = new XmlHttpRequest();
// Indicate the ability to handle a timeout.
req.timeout = 1000;
req.ontimeout = () => ...;
// ...but express the expected happy-path as a regular onload event
req.onload = () => ...;

cannot have its onload path tested (even with res.timeout(false) or res.timeout(0)), because the presence of req.timeout forces the timeout path, due to the Math.max() here: https://github.com/marvinhagemeister/xhr-mocklet/blob/master/src/MockXMLHttpRequest.ts#L217

I believe that MockXMLHttpRequest::timeout should only indicate the capacity to handle a timeout, but MockXMLHttpResponse::timeout should indicate the choice to take the timeout path.

This PR updates MockXMLHttpRequest::send() so that it only takes the timeout path when a response timeout is indicated.

joscha commented 6 years ago

👍 @marvinhagemeister can we create a release from this, please?

marvinhagemeister commented 6 years ago

yup, just published it as xhr-mocklet@1.2.3 🎉