search

arthurnn / twirp-ruby

Twirp services in Ruby
Apache License 2.0
155 stars 60 forks source link

Rack 3 errors - rack_request.body.rewind, example app #111

Closed jtippett closed 5 days ago

jtippett commented 1 year ago

The line at https://github.com/arthurnn/twirp-ruby/blob/caf2381a94721d830c47e38ea244fa1c7a8dc006/lib/twirp/service.rb#L141 causes requests to fail when set up with rack 3, as it no longer guarantees rewindability https://github.com/rack/rack/blob/8f5c885f7e0427b489174a55e6d88463173f22d2/UPGRADE-GUIDE.md?plain=1#L176

The issue can be avoided by checking first: rack_request.body.rewind if rack_request.body.respond_to?(:rewind). I note some discussion on this in the past and not sure what kind of side effects this might have. Happy to submit a PR if desired. I think dealing with it pretty important though, because the error given is very misleading, telling users they have a malformed request when actually it's a rack version issue.

There are also a bunch of updates needed for the example app to work with rack 3 - some functionality has been separated into a separate rackup gem, for example. The example app works if bundle installed from there - but copying its pattern in a newer environment will fail. Again, happy to put it in a PR but I wonder if rack 3 compat is a bigger deal than these issues.

jtippett commented 1 year ago

If it's not too annoying I'll just use this issue to post problems as I find them. Rack 3 enforces linting aggresively, and throws Rack::Lint::LintError: uppercase character in header name because it doesn't like "Content-Type: application/json", which is generated by this lib. I "fixed" it by setting ENV["RACK_ENV"] = "production" in config.ru.

Personally I think this is ridiculous and there is nothing wrong with Content-Type but anyway, that's what happens.

arthurnn commented 2 months ago

I tested the app, and it seems that twirp-ruby does not work with rack 3.x