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

POST method fails when using with new Unleash Edge service #228

Closed
mhuffsti opened this issue Aug 29, 2024 · 7 comments · Fixed by #229
Closed

POST method fails when using with new Unleash Edge service #228

mhuffsti opened this issue Aug 29, 2024 · 7 comments · Fixed by #229
Assignees

Comments

@mhuffsti
Copy link

I'm not sure if the issue is on the Edge side or within this unleash-proxy-client or maybe I should be using a completely different frontend JS client when switching from Proxy to Edge, but I'll start with this issue here.

I have the unleash-proxy-client setup and working fine and it is making its requests against an Unleash Edge server. Everything seems to be working fine. However, we wanted to switch from using GET to POST. When we set usePOSTrequests to true, this part of the code:

? JSON.stringify({ context: this.context })

builds a POST payload that takes the form of (eg, a generalized example for illustration purposes):

{"context":{"sessionId":#######,"appName":"APP_NAME_HERE","environment":"ENV_NAME_HERE"}}

However, when that request hits the Edge server, it responds with:
Json deserialize error: invalid type: map, expected a string at line 1 column ##

I did some testing, and if we remove the "context" piece and only send as the payload the contents of context, then the Edge server responds appropriately.

Question:

  • Should I be using a different client against Edge?
  • If not, then does this part of the code need to be updated to no longer inject {"context":... to the POST payload?
@chriswk
Copy link
Member

chriswk commented Sep 3, 2024

Hi @mhuffsti , sorry for the delayed response. I've made Unleash/unleash-edge#510 which should fix it, and will be released as soon as @sighphyre and myself can agree on the way forward.

@chriswk
Copy link
Member

chriswk commented Sep 3, 2024

Way forward agreed. We'll release 19.3.1 sometime today and I'll update this again when you can upgrade your Edge.

@chriswk
Copy link
Member

chriswk commented Sep 3, 2024

Hi @mhuffsti , could you please try to use 19.3.1. It worked for us, but not closing this ticket until I have confirmation from you that it works from your end.

@mhuffsti
Copy link
Author

mhuffsti commented Sep 3, 2024

Thanks for looking into this so quickly. I can confirm that it is better, yes, but maybe something more is needed in terms of the validation of the JSON POST payload on the Edge side?

Specifically, if I set usePOSTrequests to true on the proxy client, the 19.3.1 release of Edge is parsing the "context" part now.
If I set a sessionId as a string, then it seems quite happy.
However, if I let the proxy client autogenerate a sessionId, it generates a number and sends that number like this:

image

Edge then generates a 400 Bad request with this response:
Json deserialize error: invalid type: integer 299923022, expected a string at line 1 column 33

Should it be expecting a string? Or should it accept either a string or an integer?

@chriswk
Copy link
Member

chriswk commented Sep 4, 2024

It should be expecting a string, at least that's what our schema says, but I guess proxy has just been tolerating this since it's javascript. But with actual typed languages, this breaks. I'll make a PR on this repo

@chriswk chriswk linked a pull request Sep 4, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from Support rotation to Done in Issues and PRs Sep 4, 2024
@chriswk chriswk reopened this Sep 4, 2024
@github-project-automation github-project-automation bot moved this from Done to New in Issues and PRs Sep 4, 2024
@chriswk
Copy link
Member

chriswk commented Sep 4, 2024

Ok, bugfix commited, will release a 3.6.1 of the client.

@chriswk
Copy link
Member

chriswk commented Sep 4, 2024

3.6.1 released which will post sessionId as a string also if auto generated. Thanks for letting us know. Closing this issue now.

@chriswk chriswk closed this as completed Sep 4, 2024
@github-project-automation github-project-automation bot moved this from New to Done in Issues and PRs Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants