jameslnewell / xhr-mock

Utility for mocking XMLHttpRequest.
196 stars 48 forks source link

allow handlers to return a promise #10

Closed dereke closed 7 years ago

dereke commented 8 years ago

this change allows you to return a promise from a handler. this is useful when you don't want to respond to the request immediately.

For example you may want to test that some loading code is executed when the response does not return complete after 200ms (one of my use cases)

jameslnewell commented 8 years ago

Thanks for the PR! I'm currently on holidays but ping me if I haven't replied by the 27th.

dereke commented 8 years ago

@jameslnewell ping :-)

jameslnewell commented 8 years ago

Awesome! Thanks for the PR, that'll tick off my "Ability to return mocked responses asynchronously" TODO item.

One question, what should happen if the promise is rejected?

dereke commented 8 years ago

good point! I've put in some code to handle rejected promises. To start with I am just writing the status code and response, I am not sure if it should also return headers etc? Probably it should return the response in the same way as a good request, just in the error state. What do you think?

jameslnewell commented 8 years ago

I'd assume the promise rejected with an error, not a response (you'd still resolve() a 500 response). I'd probably do the same as this case https://github.com/jameslnewell/xhr-mock/blob/master/lib/MockXMLHttpRequest.js#L170-L172

dereke commented 8 years ago

yeah you are right! So it seems unlikely that you would ever call failure as you would presumably catch any errors internally and return the error response. I suppose if you were feeling lazy you would just pass the failure callback to another promise, or call it inside a try/catch:

doSomething().then(function(){
  success(res.status(200));
}).catch(failure)

// or

try { 
 doSomething(); 
  success(res.status(200))
}
catch (e) {
  failure(e)
}

in that case we could check for an Error object and return status 500 with the error message as the body.

does that sound OK?

jameslnewell commented 8 years ago

Yep I agree, failure()/reject() would rarely (if ever) be called by the user of the library, but there is the rare case they might accidentally cause an error to be thrown in a promise chain - but we should handle it to alert the user that it wasn't the response they expected.

I think that in that case it shouldn't return a 500 status (because if the user is testing a 500 the test will pass). Using the same as here will make it work similar to not being able to connect at all.

jameslnewell commented 8 years ago

@dereke where'd we get up to on this?

If you remove setting the status and body I think it'll be good to go? https://github.com/jameslnewell/xhr-mock/pull/10/files#diff-25a66c4ad09cda58c62379ebbee456e6R178

jameslnewell commented 7 years ago

Going to close this due to in-action. Note, the original use case is already supported.

In README.md:

.timeout() : bool|number

Get whether the response will trigger a time out.

.timeout(timeout : bool|number)

Set whether the response will trigger a time out. timeout defaults to the value set on the XHR object.

Happy to continue down this road if you want to finalise the last few things.

dereke commented 7 years ago

ah I forgot about this as I've headed down a different route. you might be interested https://github.com/dereke/vinehill