Set cookies issue with corking

[lewispham]

Applying corking is quite affecting my current application architecture as it's no longer allowed to call res.writeHeader within the services. Instead, I need to collect all headers then send them all at once within res.cork. But collecting multiple Set-Cookie headers will still be problematic though.

const responseHeaders = {
    "Set-Cookie": [
        "show_pop=true; Expires=Tues, 27 Nov 2016 07:45:00 GMT; Domain=foo_test.com; SameSite=Strict",
        "language=en; Expires=Tues, 27 Nov 2016 07:45:00 GMT; Domain=foo_test.com; SameSite=Strict"
    ]
};
res.cork(() => {
    for(const cookie of responseHeaders["Set-Cookie"]) {
        res.writeHeader("Set-Cookie", cookie);
    }
    res.end();
});

Did I misunderstood the feature or its usage? Otherwise, do you have a recommendation for me in this case?

[e3dio]

When the response is async I use these functions to gather the Status and Header values from different middleware and write them at the end, because the Status needs to be written before the headers and you want to cork to write status, headers, and body at same time:

const setStatus = (res, status) => res.status = status;
const setHeader = (res, name, value) => (res.headers ??= []).push(name, value);
const end = (res, body, status = res.status) => !res.ab && res.cork(() => {
   if (status) res.writeStatus(status);
   if (res.headers) for (let i = 0; i < res.headers.length;) res.writeHeader(res.headers[i++], res.headers[i++]);
   res.end(body);
});

usage:

app.get('/', async res => {
   res.onAborted(() => res.ab = true);
   await doAsync();
   setHeader(res, 'Set-Cookie', 'name=value');
   await doAsync();
   end(res, 'body', '304');
});

[lewispham]

Thanks for the idea. So basically we'll need to wrap response with an extra layer to change the behavior of writeHeader so that they'll be only truly send when end is called. I think it's best if uws.js can handle this internally.

@alexhultman Is there any chance that the above suggestion will be handled by uws.js internally? I think it might be better for uws.js to work like the above regardless the corking.

[e3dio]

I think it purposefully provides a minimal API for best performance, if you want to cache headers between async middleware you can add these functions to do that, and you can edit to how you want it to work. If you make the library do it by default it adds an extra cache that is not necessary when you do direct synchronous response. I could see making res.onAborted(()=>res.aborted=true) get called by default when you do async, that would save me extra code in all my async handlers

[lewispham]

Well nodejs is all about async. 99% of the time the response will be sent async. In other words, the current behavior of corking is not quite useful. As a result, most developers will end up applying the non corking approach. It's up to the lib owner anyway.