HEAD call useless without resolveWithFullResponse
zcei opened this issue · 8 comments
As a HEAD
call is a GET
call without response body, it's pretty useless without the resolveWithFullResponse
option, as you get an empty string all the time.
I personally would vote for having res.headers
as the default parameter passed to .head()
calls, or to at least set resolveWithFullResponse
to true
by default.
Or is it intended to preserve the same behavior in all convenience functions, even if it's nonsense to a particular function?
Good point @zcei !
Indeed, I do prefer having the same default settings for all convenience functions because it is less confusing for the users. Nonetheless could you rephrase your proposed change? I don't understand what you mean by "having res.headers as the default parameter passed to .head() calls".
Right away you may make use of the .defaults()
feature:
var rp = require('request-promise');
var rpWithFullResp = rp.defaults({ resolveWithFullResponse: true });
rpWithFullResp.head(...);
rp.get(...);
Uh, saw the defaults
but didn't read properly. I thought it'd set it on rp
, thanks!
That said, it's actually sufficient for me, consistency has a lot of value worth keeping.
The proposed change would be one .then()
function as an interceptor for HEAD
calls.
e.g.:
.then(function extractHeader(res) {
return res.headers;
})
I like your idea. This improves convenience and doesn't really introduce inconsistency in this case.
Would it be sufficient to add another else if
branch?
if (isFunction(self._rp_options.transform)) {
try {
self._rp_resolve(self._rp_options.transform(body, response));
} catch (e) {
self._rp_reject(e);
}
} else if (self._rp_options.resolveWithFullResponse) {
self._rp_resolve(response);
+ } else if (response.request.method === 'HEAD') {
+ self._rp_resolve(response.headers);
} else {
self._rp_resolve(body);
}
or would you directly want to implement something like RP$middlewareInterceptor
, to possibly treat other special cases?
I already started the implementation and chose to add it directly because that should be much more performant:
if (isFunction(self._rp_options.transform)) {
try {
self._rp_resolve(self._rp_options.transform(body, response));
} catch (e) {
self._rp_reject(e);
}
} else if (self._rp_options.resolveWithFullResponse) {
self._rp_resolve(response);
} else {
- self._rp_resolve(body);
+ self._rp_resolve(self._rp_options.method && self._rp_options.method.toUpperCase() === 'HEAD' ? response.headers : body);
}
However, an interceptor might be a very useful idea. Please have a look at the transform function and give me feedback whether this already covering everything you imagine or if we should extend its capabilities or if a new interceptor mechanism would allow to cover new use cases. Thanks a lot!
Transform seems fine for me.
Then we could modify the RP$initInterceptor
:
if (isString(options.method)) {
options.method = options.method.toUpperCase();
+ options.transform = options.transform || defaultTransformations[options.method];
}
Obviously with an defaultTransformations
object like:
var defaultTransformations = {
HEAD: extractHeaders
};
Then the transformation stays undefined
as it currently is, when we don't need any further treatment.
Thanks @zcei that is a really clean solution!
I will take it from here and let you know when I released the next version.
I just released version 1.0.0 which fixes this issue. Thanks for providing the solution in your last comment @zcei which I just had to apply.