Multiple cookie headers are not supported for Remix platform

[alfredsgenkins]

Hey!

I am not 100% sure if that's an issue with SST. All I know at this point, is that I have a Remix package, implementing the following logic to set multiple cookies: https://github.com/remix-run/remix/issues/231#issuecomment-2227992952.

On local (with remix-serve) it works well, it outputs the multiple headers in response:

In production, or deployed, only first cookie is set:

I have found some information on special format, that lambdas should follow to declare cookies: https://stackoverflow.com/a/68994753.

I will continue my investigation.

[luke-rucker]

fixed with #702. it's in v0.0.539

[alfredsgenkins]

After upgrading to this version (to v.0.0.540) all my functions now fail with following error:

ERROR   Invoke Error    
{
    "errorType": "TypeError",
    "errorMessage": "response.headers.getSetCookie is not a function",
    "stack": [
        "TypeError: response.headers.getSetCookie is not a function",
        "    at Runtime.handler (file:///var/task/build/server.mjs:113737:33)",
        "    at async Runtime.handleOnceStreaming (file:///var/runtime/index.mjs:1206:26)"
    ]
}

Update 1

My app Lambda function appears to be created by SST and runs Node.js 20.x. Tried to reproduce on my local, with request.headers object. I am using Node v20.14.0. I am also getting this issue:

While the MDN says it is fully supported since Node 19.7.0: https://developer.mozilla.org/en-US/docs/Web/API/Headers/getSetCookie#browser_compatibility

Update 2

Just figured the request / response headers property is coming from Remix custom @remix-run/web-fetch package. Specifically, here is the Headers implementation: https://github.com/remix-run/web-std-io/blob/7a8596e6fb6d9aed6268145ebe43005d9e6ca94d/packages/fetch/src/headers.js#L52. And it, (surprise, surprise!) does not implement the getSetCookie method.

I am using the latest Remix:

"@remix-run/node": "^2.10.2",
"@remix-run/react": "^2.10.2",

On what version of Remix have you tested your implementation? It appears that currently, the Remix does not provide getSetCookie implementation on headers object.

Update 3

It appears Remix is no longer shipping polyfills by default. Based on this issue, the Node 20 users do not require polyfills. The problem could only appear for users who have installed globals in vite, like I had:

import { installGlobals } from "@remix-run/node";

installGlobals();

I will try to remove this line and see if it solves the issue. I believe the SST should then mention that installGlobals are not supported.

[luke-rucker]

Ah, sorry about that. I'll open another PR that doesn't require getSetCookie

[alfredsgenkins]

I have removed the above mentioned installGlobals() command. However, I am still getting the same single cookie result (last set-cookie header) as before:

While the expected result from the code bellow is two set-cookie headers.

class {
  async getLogoutHeaders() {
    const headers = new Headers();

    headers.append(
      "Set-Cookie",
      await idTokenCookie.serialize("", { maxAge: 0 }),
    );

    headers.append(
      "Set-Cookie",
      await refreshTokenCookie.serialize("", { maxAge: 0 }),
    );

    return headers;
  }
}

Update 1

Interestingly, my bitbucket pipelines deploy functions that break with response.headers.getSetCookie is not a function but my local deploy to SST does not. I found it weird, decided to look what could be causing this.

I looked into SST code, and found the following lines: https://github.com/sst/ion/blob/d1dc34806a0dbbb88c5fe9444180bc478b1703be/platform/functions/remix-server/polyfill.mjs#L24 https://github.com/sst/ion/blob/d1dc34806a0dbbb88c5fe9444180bc478b1703be/platform/src/components/aws/remix.ts#L601-L610 https://github.com/sst/ion/blob/d1dc34806a0dbbb88c5fe9444180bc478b1703be/platform/src/components/aws/remix.ts#L616

So, it appears that SST always installs globals. But how then I got my functions to work, once deployed from local? Strange...

Update 2

I have downloaded the source code of the lambda function, which worked when deployed from my local. It appears it does not contain the changes you made in 0.0.539 release. I looked into this further, and it appears, that the .sst directory still contains the outdated Remix regional server code:

I suspect that the reason why the code was working on local is that the old version of SST got deployed, while in pipelines, the .sst folder was not present, and got generated on deploy, containing the latest version of the source code.

Looks like the pnpm up sst@ion -r which I run from my workspace root did not actually update the .sst folders. Based on the docs, I should have used sst upgrade 0.0.540 command. It's a little bit inconvenient, since I have to run it from all 6 of my sst managed Remix apps.

Potentially, I should have used SST Monorepo https://ion.sst.dev/docs/examples/#aws-monorepo feature, but I did not manage to learn how can I configure multiple SST apps inside.

[luke-rucker]

just opened a pr that should fix this, polyfills or not: #709.

as far as your issues with different behavior locally vs in ci, the cli recently started being packaged along with the npm package. so you can now get rid of your global installation, and work with the cli through pnpm sst. it's more to type but more convenient imo as the cli will always match the sdk version, preventing any headaches like you experienced

[luke-rucker]

the fix of the fix is in v0.0.545 :)

[alfredsgenkins]

It did solve the issue! However, my headers are sent +1 times. I think set-cookie must be removed from headers, once converted to cookies. For example, I tried sending id_token and refresh_token cookies. Using this code:

class {
  async getLogoutHeaders() {
    const headers = new Headers();

    headers.append(
      "Set-Cookie",
      "...value1..."
    );

    headers.append(
      "Set-Cookie",
      "...value2..."
    );

    return headers;
  }
}

Instead, I got these:

1. 
2. 
3. 

There should be just 2 of them, not 3. I do not think the issue is critical anymore, yet I think it would be best if we address this inconsistency too.

[jayair]

Would appreciate a PR1