Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$return a "cookies" object has no effect in V2 - emit warning #284

Closed
kepikoi opened this issue Feb 20, 2020 · 6 comments
Closed

$return a "cookies" object has no effect in V2 - emit warning #284

kepikoi opened this issue Feb 20, 2020 · 6 comments

Comments

@kepikoi
Copy link

kepikoi commented Feb 20, 2020

Using node runtime it is now possible to send multiple cookies like documented here Azure/azure-functions-host#2737

E.g.:

module.exports = async function (context, req) {
    context.res = {
        body: "Hello",
        cookies: [{
            name: "kookay",
            value: "2355tgrbf"
        }, {
            name: "kookay2",
            value: "rfdvt54"
        }]
    };
};

/** 
Cookies present in the response:

GET http://localhost:7071/api/hello
200
23ms
Network
Request Headers
Response Headers
Date: Thu, 20 Feb 2020 11:46:25 GMT
Content-Type: text/plain; charset=utf-8
Server: Kestrel
Content-Length: 5
Set-Cookie: kookay=2355tgrbf; path=/
Set-Cookie: kookay2=rfdvt54; path=/
Response Body
**/

But it doesn't work using the $return binding:

module.exports = async function (context, req) {
    return {
        body: "Hello",
        cookies: [{
            name: "kookay",
            value: "2355tgrbf"
        }, {
            name: "kookay2",
            value: "rfdvt54"
        }]
    };
};

/**
No response cookies:
 
GET http://localhost:7071/api/hello
200
15ms
Network
Request Headers
Response Headers
Date: Thu, 20 Feb 2020 11:51:48 GMT
Content-Type: text/plain; charset=utf-8
Server: Kestrel
Content-Length: 5
Response Body
**/
@ahmelsayed ahmelsayed transferred this issue from Azure/azure-functions-core-tools Feb 20, 2020
@ahmelsayed
Copy link

/cc @mhoeger for FYI

I can repro that using the function from the issue with this function.json

{
  "bindings": [
    {
      "authLevel": "function",
      "type": "httpTrigger",
      "direction": "in",
      "name": "req"
    },
    {
      "type": "http",
      "direction": "out",
      "name": "$return"
    }
  ]
}

@ahmelsayed
Copy link

Actually I see a test here for that scenario, so I'm not sure if it's a core-tools specific issue?

@mhoeger
Copy link
Contributor

mhoeger commented Feb 20, 2020

@kepikoi - what version of functions are you using?

@kepikoi
Copy link
Author

kepikoi commented Feb 21, 2020

@ahmelsayed I just tested the $return case on my azure functions instance and it doesn't seem to work there either. So is it then https://github.com/Azure/azure-functions-host specific?

@mhoeger

PS C:\> func -v
2.7.2184
PS C:\> node -v
v10.19.0
PS C:\> npm -v
6.13.4
PS C:\> [System.Environment]::OSVersion.Version

Major  Minor  Build  Revision
-----  -----  -----  --------
10     0      18362  0

@mhoeger
Copy link
Contributor

mhoeger commented Feb 21, 2020

@kepikoi - it looks like this is the root cause: #228 (sorry took me a bit to remember!)

In V2, we have this issue where HTTP output through $return does not go through the same code path to create an http request. We couldn't change it in V2 because it is a breaking change (there are some weird but nice things you can do in V2 like returning a string and that will directly end up as the body).

Recommendation is to either not use the $return binding or switch to V3. One thing we could do in V2 is to emit a warning if we see someone using cookies + $return. So I'll keep this issue open!

@mhoeger mhoeger changed the title $return a "cookies" object has no effect $return a "cookies" object has no effect in V2 - emit warning Feb 21, 2020
@mhoeger mhoeger added this to the Triaged milestone Feb 21, 2020
@ejizba ejizba removed this from the Triaged milestone Nov 15, 2021
@ejizba
Copy link
Contributor

ejizba commented Jan 28, 2022

Looks like this was already fixed in v3. Only thing left is this:

One thing we could do in V2 is to emit a warning if we see someone using cookies + $return. So I'll keep this issue open!

But since v2 is pretty old at this point, I will go ahead and close.

@ejizba ejizba closed this as completed Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants