Add support for gleam v1.0.0 #20
Conversation
gleam.toml
Outdated
| @@ -14,7 +14,7 @@ links = [ | |||
| gleam_stdlib = "~> 0.31" | |||
| gleam_http = "~> 3.0" | |||
| gleam_otp = "~> 0.7" | |||
| cowboy = "~> 2.0" | |||
| cowboy = "~> 2.10.0" | |||
There was a problem hiding this comment.
You've downgraded the requirement here to be very pessimistic. Before it was installing 2.x, now it's 2.10.x. Could you revert this please? 🙏
There was a problem hiding this comment.
Yeah I addressed this in the third dot point, but basically with cowboy 2.11.0 they changed the way they handled cookie headers. In particular, you get this error when running the tests:
=ERROR REPORT==== 17-Mar-2024::16:54:14.917416 ===
Ranch listener {gleam_cowboy,#Ref<0.2594036081.850657288.229963>}, connection process <0.573.0>, stream 1 had its request process <0.574.0> exit with reason {response_error,invalid_header,'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'} and stacktrace [{cowboy_req,reply,4,[{file,"/Users/bradlewis/repos/cowboy/build/dev/erlang/cowboy/src/cowboy_req.erl"},{line,813}]},{gleam_cowboy_native,init,2,[{file,"/Users/bradlewis/repos/cowboy/build/dev/erlang/gleam_cowboy/_gleam_artefacts/gleam_cowboy_native.erl"},{line,22}]},{cowboy_handler,execute,2,[{file,"/Users/bradlewis/repos/cowboy/build/dev/erlang/cowboy/src/cowboy_handler.erl"},{line,37}]},{cowboy_stream_h,execute,3,[{file,"/Users/bradlewis/repos/cowboy/build/dev/erlang/cowboy/src/cowboy_stream_h.erl"},{line,306}]},{cowboy_stream_h,request_process,3,[{file,"/Users/bradlewis/repos/cowboy/build/dev/erlang/cowboy/src/cowboy_stream_h.erl"},{line,295}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,241}]}]
I'll have a quick look into seeing if I can update the lib to support cowboy_req:set_resp_cookie instead to keep the requirement as is.
For context, it's the same problem as described for plug elixir-plug/plug_cowboy#98
There was a problem hiding this comment.
Roger. Let's go for ~> 2.11 to be on the right side of that breaking change.
There was a problem hiding this comment.
After a bit of trial and error I managed to get it to work. I needed to update the CowboyRequest object directly, which is the path that was suggested here: ninenines/cowboy#1624 (comment) and the solution chosen by the elixir cowboy project.
This lets us keep the requirement back to just ~> 2.0. Let me know what you think of this.
FWIW I did look into using the cowboy_req:set_resp_cookies function, but I was unable to get that to work, both due to parsing the cookie string dropping some information, and not being able to pass the opts easily.
Cowboy 2.11.0 removed the option of setting cookie headers in `cowboy_req:reply`, `cowboy_req:set_resp_header` or `cowboy_req:set_resp_headers` To solve this we update the CowboyRequest headers directly, as suggested here: ninenines/cowboy#1624 (comment)
|
Yeah this works with all of the older versions. Though after testing these older versions, there are other issues with versions < 2.5.0, so I'll update the dependencies to reflect that. |
This PR contains the necessary fixes to have
gleam_cowboywork with gleam v1.0.0.In particular it changes:
gleam/mapwithgleam/listgleam/bit_builderwithgleam/bytes_buildercowboyto2.10due to some changes with the management of cookie headers in the cowboy library. I wasn't able to figure out how to allow forcowboy> 2.0 but < 2.11, so let me know if there's a better way to do this!I've also updated the example in the README reflect these changes.
Let me know what you think!
Resolves: #19