Require the webrick handler when webrick is available

[Earlopain]

Followup to #23.

Even when users do add webrick to their gemfile, the handler is not available. This means that they either need to do that themselves, or their dependencies need to be updated. A quick search shows a handful of matches: https://github.com/search?q=%2FRackup%3A%3AHandler%3A%3AWEBrick%2F+lang%3Aruby+-is%3Afork&type=code

I noticed that capybara already does require the handler here but it doesn't have to. Just requiring rackup previously made it available. I think it's a fair assumption that there is code out there that would be impacted by this.

[ioquatix]

I understand the value of the PR.

However, the entry point should be:

https://github.com/rack/rackup/blob/main/lib/rackup/handler.rb#L40-L56

Rackup::Handler.get(:webrick)

This will return whatever has been registered as that handler, and would handle compatibility issues w.r.t. the older rack vs rackup paths.

Thoughts?

[Earlopain]

Yes, that's true. It should be used but it is not always the case. A github issue search yields yabeda-rb/yabeda-prometheus#32 which this would help with. I think it's reasonable to fix breakage from a minor version change if possible

Going through with this would soften the blow a bit but personally I'm not affected.

[ioquatix]

Yes, I agree with the compatibility angle, so I'm probably okay to merge this PR just on that basis.

Out of curiosity, is there any way we can steer people in the right direction? e.g. Rackup::Handler.get(:webrick)? I feel if this was just used initially, e.g. in capybara, a lot of pain would have been avoided?

[Earlopain]

Out of curiosity, is there any way we can steer people in the right direction?

I'm not sure how you would achieve that tbh. I have no idea how puma handles it, if the handler is required through the main entrypoint (probably not if I had to guess), it's just a bit unfortunate that for webrick that used to be the case.

If capybara used Handler.get, I guess it would throw an error with a slightly different backtrace but overall still the same. The implementation on the capybara side would be neater but users without webrick are out of luck regardless. Still a LoadError in the end.

[Earlopain]

It does raise a somewhat interesting question. psych recently dropped a require for stringio, breaking downstream code relying on it being available. Is this something psych should fix? I think not.

But rackup seems to be pretty much in maintanence mode so I'd guess not much going on here in the future. The last 2 years were already very sparse. So I don't mind fixing it here in this case.

[ioquatix]

I agree with your sentiment regarding psych and stringio.

I don't want to cause too many downstream problems either though.

We should encourage downstream to use Rackup::Handler.default if they are trying to get a server instance, or require 'rackup/handler/webrick' if they want to use Rackup::Handler::WEBrick directly.

[ioquatix]

I am okay with this so I have merged it.

However, I'm thinking about downstream users who don't want webrick.

I wonder if we could use autoload instead?

[ioquatix]

Released in v2.2.1.

@fxn do you have any thoughts on how to improve this? (autoload etc).

[Earlopain]

I started out with gem "webrick" to detect it but it would show the ruby warning on 3.0

I guess you could do a combination of the two? Do gem "webrick" to start out with and then do a require to be sure?

Edit: Ah, nevermind. I completely missed the mark with this. It will not change anything about what you've written. If you go with autoloading, you should make sure that loading the handler doesn't emit the ruby warning during it.