Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

requestFragment type definition does not match implementation #327

Open
atkinchris opened this issue May 8, 2020 · 0 comments · May be fixed by #328
Open

requestFragment type definition does not match implementation #327

atkinchris opened this issue May 8, 2020 · 0 comments · May be fixed by #328

Comments

@atkinchris
Copy link

atkinchris commented May 8, 2020

The type definition for requestFragment does not match the implementation.

It is a single function that takes 5 arguments and returns a promise.

, requestFragment?: (filterHeaders: (attributes: Attributes, req: IncomingMessage) => object, url: Url, attributes: Attributes, req: IncomingMessage, span?: Span) => Promise<ServerResponse>

However, the implementation is a function that takes a single argument, and returns a function that takes 4 arguments and returns a promise.

module.exports = filterHeaders => (
fragmentUrl,
fragmentAttributes,
request,
span = null
) =>
new Promise((resolve, reject) => {

The response type is ServerResponse, but in on('response', response => ...) is an IncomingMessage

fragmentRequest.on('response', response => {
if (response.statusCode >= 500) {
reject(new Error('Internal Server Error'));
} else {
resolve(response);
}
});

Argument url is an attribute from the tag, and therefore a string, not an instantiated URL object.
https://github.com/atkinchris/tailor/blob/3123dc4d182aef299e7ec1385005cbcf1368134b/lib/fragment.js#L116-L118

I think the correct type definition should be:

(filterHeaders: (attributes: Attributes, req: IncomingMessage) => object) => (url: string, attributes: Attributes, req: IncomingMessage, span?: Span) => Promise<IncomingMessage> 
atkinchris added a commit to atkinchris/tailor that referenced this issue May 8, 2020
This corrects the type definition for requestFragment, which does not match it's implementation. This resolves zalando#327 and supercedes https://github.com/zalando/tailor/pull/297/files.

This also correct initialisation of custom `requestFragment` functions, which otherwise are not invoked to close over `filterHeaders`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant