Add request interception hooks #92
Conversation
|
Build fails because the new version of |
|
Can we just globally inject something like https://www.npmjs.com/package/es6-promise to make it happy? It won't add the bluebird sugar methods so won't mask any errors related to that. |
| interceptResponseOrError(Promise.reject(responseError)) | ||
|
|
||
| interceptRequestOrError = (initialPromise) -> | ||
| Promise.resolve( |
There was a problem hiding this comment.
Why do we need this wrapper? The inner thing is already a promise
There was a problem hiding this comment.
I think this the result of reduce could be a non-bluebird promise, if a hook internally returns a native promise - this makes sure we definitely always get a bluebird promise at the end.
lib/request.coffee
Outdated
| interceptResponseOrError = (initialPromise) -> | ||
| Promise.resolve( | ||
| exports.interceptors.slice().reverse().reduce (promise, interceptor) -> | ||
| promise.then(interceptor.response, interceptor.responseError) |
There was a problem hiding this comment.
if both are null-ish will p.then(null, null) resolve with the original promise value?
There was a problem hiding this comment.
OK it works but shows a warning: https://monosnap.com/file/1uCMR5wTIKJtfoxQwm90SmJtKosfTE.png
So would change to
{ response, responseError } = interceptor
return if (response or responseError) then promise.then(interceptor.response, interceptor.responseError) else promise`
(and same below)
There was a problem hiding this comment.
Done - destructuring here is a great idea.
lib/request.coffee
Outdated
| # reverse order for responses. | ||
| # | ||
| # @example | ||
| # resin.interceptors.push( |
There was a problem hiding this comment.
Good spot. I've gone for request in the end, for consistency with the other examples.
tests/hooks.spec.coffee
Outdated
|
|
||
| it 'should be able to asynchronously change a request before it is sent', -> | ||
| request.interceptors[0] = request: (request) -> | ||
| Promise.fromCallback (callback) -> |
There was a problem hiding this comment.
NIT but can use http://bluebirdjs.com/docs/api/delay.html
| promise = request.stream | ||
| url: 'https://500.com' | ||
|
|
||
| m.chai.expect(promise).to.be.rejectedWith('caught error') |
|
All done - one comment on the I fixed the build by putting a patch in to |
|
Yeah the reasoning behind using |
While working on https://github.com/resin-io/resin-ui/issues/400, it became clear the SDK currently isn't a usable replacement for our api calls in the UI, because it can't use Angular's built-in
$httpclient internally. We depend on all requests going through that$httpclient so we can intercept them all in various places, e.g. to support offline status monitoring.This PR adds Angular-inspired request/requestError/response/responseError hooks to resin-request, so that they can be exposed on resin-sdk shortly. These should have the same semantics as Angular's, which seem as good a model as any.