`res.headers["set-cookie"]` can be of type `string`, which is disallowed according to `res.headers`'s type `IncomingHttpHeaders`
tremby opened this issue · 7 comments
I'm making a response which is getting either zero or one or two set-cookie headers back. I need to do something with them.
const { res } = await request<string>(url, { /* ... */ });
// Typescript claims res is a RawResponseWithMeta.
const headers = res.headers;
// Typescript claims headers is an IncomingHttpHeaders, which urllib imports from node:http
const setCookies = headers["set-cookie"];
// Typescript claims setCookies is string[] | undefined
for (const setCookie of setCookies ?? []) {
// For particular requests, if I try to do something here with setCookie,
// I get errors from my cookies library.
// In those cases, if I inspect setCookie I see that it is just a single letter.
}
// In fact, in those cases:
console.log(typeof setCookies); // string -- a single headerThe broken cases are when exactly one set-cookie header was in the response.
Looking at node's IncomingHttpHeaders type via package @types/node, it seems it has various properties defined as optional and type string | undefined, and one, set-cookie in particular, is optional and type string[] | undefined. On top of that it allows any other arbitrary properties and they are of type string | string[].
It seems to me that either there's an upstream bug (in undici?), or some special handling needs to be added to HttpClient.ts for the set-cookie header.
Looking at undici's code now, it looks like their IncomingHttpHeaders type is not from node:http; it's homegrown. It's just Record<string, string | string[] | undefined>. That's not compatible with urllib's (node:http's) IncomingHttpHeaders type.
http server can response set-cookie headers many times.
Yes, I know.
I'll try to explain differently.
If the server sends more than one set-cookie header, res.headers["set-cookie"] is an array of strings. This is fine.
If the server sends just one set-cookie header, res.headers["set-cookie"] is a single string. This disagrees with the typescript type associated with res.headers["set-cookie"], which is string | undefined (which it gets from IncomingHttpHeaders from node:http). string[] is not assignable to string | undefined. So either the types are wrong, or there is a bug in the code. This means I can get errors in my code which Typescript cannot see.
Undici can also return a single string for that property, but that is not a problem since it reports the types differently. It associates type string | string[] | undefined with that property. It is not using IncomingHttpHeaders from node:http, but rather its own type which it also calls IncomingHttpHeaders. The two IncomingHttpHeaders types are not compatible.
I would follow the undici IncomingHttpHeaders fix it.
https://github.com/nodejs/undici/blob/b0d3ca7701766b6cb9ebb8fed7dd08d3a684b91b/types/header.d.ts#L4
the undici IncomingHttpHeaders is a simple version of node:http IncomingHttpHeaders, I think the node:http IncomingHttpHeaders is better and more types defines.
I agree. If you see the issue I linked to above, you'll see that at some point they planned to switch to node's type, but as far as I can tell they never got around to it.
But the fact is, what you're providing on res.headers is undici's type, but in your types you're declaring that it's node's type. You either need to fix the data so it matches the node type, or change the type declarations to be the same as undici's. Maybe you can import the relevant type straight from undici rather than recreating it; that way you shouldn't then need to change it again once they change their types.
I suppose a 3rd option would be to provide a patch to undici so that they are using node's types. But they wouldn't release that until a major version change.