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

webrick dependency: crashes CI: Also contact author of rackup-2.2.0 to add webrick into its gemspec" #26

Closed
jrochkind opened this issue Nov 7, 2024 · 14 comments

Comments

@jrochkind
Copy link

ruby 3.3.4, rails 7.2.1.1

After updating some dependencies incluidng rackup from 2.1.0 to 2.2.0.

While my app seems to work actually booting it, when I run CI I get all sorts of errors related to (apparently?) rackup and webrick.

Starts with what looks like a warning, but it winds up with apparently an infinite loop in the stack somehow (related to bootsnap and/or zeitwerk, is it really their bug?), and crashes everything.

/home/runner/work/scihist_digicoll/scihist_digicoll/vendor/bundle/ruby/3.3.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:27: warning: webrick was loaded from the standard library, but is not part of the default gems since Ruby 3.0.0. Add webrick to your Gemfile or gemspec. Also contact author of rackup-2.2.0 to add webrick into its gemspec.

from /opt/hostedtoolcache/Ruby/3.3.4/x64/lib/ruby/3.3.0/bundled_gems.rb:74:in `block (2 levels) in replace_require'
	from /home/runner/work/scihist_digicoll/scihist_digicoll/vendor/bundle/ruby/3.3.0/gems/bootsnap-1.[18](https://github.com/sciencehistory/scihist_digicoll/actions/runs/11726834138/job/32666437115?pr=2776#step:11:19).4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:27:in `require'
	from /home/runner/work/scihist_digicoll/scihist_digicoll/vendor/bundle/ruby/3.3.0/gems/zeitwerk-2.7.1/lib/zeitwerk/core_ext/kernel.rb:34:in `require'
	from /home/runner/work/scihist_digicoll/scihist_digicoll/vendor/bundle/ruby/3.3.0/gems/rackup-2.2.0/lib/rackup/handler/webrick.rb:7:in `<main>'

I am not even intentionally using webbrick. The problem is that something (zeitwerk, automatically?) is trying to require rackup/handler/webbrick even though I'm not using it, which then tries to require webrick and crashes things.

I see the webrick dependency was removed in 2.2.0, via #23 .

It may be this is causing unanticipated problems?

@ioquatix
Copy link
Member

ioquatix commented Nov 7, 2024

@fxn do you have any idea what is going on?

@Earlopain
Copy link
Contributor

Earlopain commented Nov 7, 2024

The way the warning mechanism from ruby is works is ... unfortunate. zeitwerk and bootsnap hook into require, same as ruby does itself to produce this warning. I'm somewhat sure that the frame will be improved in 3.3.5 or 3.3.6, there have been recent commits in that regard since some things keep breaking

The actual issue will be one frame above, in your case likely from this piece of code, causing capybara to load webrick: https://github.com/sciencehistory/scihist_digicoll/blob/e42be55d641746267c22c7aa8359556608facab4/spec/rails_helper.rb#L41

Capybara should rescue the LoadError, same as for puma and not assume that webrick will be available when rackup is: https://github.com/teamcapybara/capybara/blob/0480f90168a40780d1398c75031a255c1819dce8/lib/capybara/registrations/servers.rb#L7-L19

@jrochkind
Copy link
Author

jrochkind commented Nov 7, 2024

Aha, than you @Earlopain . I could have sworn I grepped my code for Capybara.server and found nothing, but I must have gotten confused.

So there was still something weird going on -- I had a thousand+ line stack trace, that repeated those same lines over and over again, without getting to the line you pulled out. Something (about zeitwerk and/or websnap?) was causing an infinite loop here of some kind, making it hard to figure out what was going on.

But that isn't rackup's problem, probably.

At any rate, if I actually set Capybara.server to :webrick, then I now need to add webrick to my gemfile, which makes some kind of sense, and I think is the intended outcome of 2.2.0 (doing that on a minor version is a choice, but one already made!). So no issue here?

@ioquatix
Copy link
Member

ioquatix commented Nov 7, 2024

Does Capybara default to webrick? Then it should add a dependency on webrick. Or it shouldn't default to anything and consider using Rackup::Handler.default (and fail if there is no reasonable default).

def self.default
if rack_handler = ENV[RACKUP_HANDLER]
self.get(rack_handler)
elsif rack_handler = ENV[RACK_HANDLER]
warn "RACK_HANDLER is deprecated, use RACKUP_HANDLER."
self.get(rack_handler)
else
pick SERVER_NAMES
end
end

@Earlopain
Copy link
Contributor

Does Capybara default to webrick

Capybara defaults to puma https://github.com/teamcapybara/capybara/blob/0480f90168a40780d1398c75031a255c1819dce8/lib/capybara.rb#L253-L256 (but it recommends webrick when you don't want to add puma to your bundle)

At any rate, if I actually set Capybara.server to :webrick, then I now need to add webrick to my gemfile, which makes some kind of sense, and I think is the intended outcome of 2.2.0 (doing that on a minor version is a choice, but one already made!). So no issue here?

Yeah, if you want to stick with webrick for capybara, you'd have to add it to your gemfile yourself. I don't think I'm getting to far of when I say there is no way that rackup is responsible for the SystemStackError you are getting. I would be curious about more of the stacktrace if you have it. Is there a command I can use locally to reproduce?

@fxn
Copy link

fxn commented Nov 7, 2024

Hey! Zeitwerk decoration is super thin, basically if no loader manages the argument, it forwards the call. But you see it in the stack trace.

These warnings are confusing. The first implementation assumed a certain call stack, but in Ruby you just cannot assume much, since things are open by design. That was later improved, but the root limitation cannot be avoided I guess.

@jrochkind
Copy link
Author

OK, I can't explain what I'm seeing in my app. With rackup 1.2.0, accidentally choosing Capybara.server = :webrick, and without webrick in the bundle.

here's a link to the apparently (infinitely?) looping stack trace, that appears in the middle of the rspec progress output (ie, not in test failure summary but printed to console in the midst or progress. You probably didn't need all 2100 lines of it since it looks like it just repeats, but just to make sure I'm not missing anything....

https://gist.github.com/jrochkind/8372c16b549020d59526cb2d4348256c

@ioquatix
Copy link
Member

ioquatix commented Nov 7, 2024

@Earlopain can I assign this issue to you?

@Earlopain
Copy link
Contributor

Earlopain commented Nov 7, 2024

gist with errors

Ah, right. I thought you meant one single backtrace but this seems to just be many smaller ones. Can't tell why it would do that but it is just an effect from the same cause.

can I assign this issue to you?

I guess? But I don't think there is much to do here:

  • A ruby bug hides the proper error location ~
    (capybara rackup in this case). I'm pretty sure that is fixed in 3.3.5 though
  • Capybara is not prepared to handle the absence of webrick, when the user explicitly asks for it. I would file an upstream issue for this.

@jeremy
Copy link
Member

jeremy commented Nov 7, 2024

There's no infinite loop in the output. Each test is booting up Capybara which is trying to require "rack/handler/webrick", rescuing LoadError, then trying to require "rackup/handler/webrick", and failing to load webrick.

Capybara will need to rescue LoadError again and give an informational message about adding webrick to Gemfile, like it does for Puma.

So there's nothing to do here. Good visibility into the headaches that churn like this induces.

@jeremy jeremy closed this as completed Nov 7, 2024
@jrochkind
Copy link
Author

Ah, i see. Thanks.

I just switched back to Capybara.server = :puma myself, and am good.

I wonder if someone ought to report this Capybara?

@Earlopain
Copy link
Contributor

I will create a PR to capybara for a better error message tomorrow, if no one has beaten me to it yet by then.

@ioquatix
Copy link
Member

ioquatix commented Nov 8, 2024

Thanks everyone!

@Earlopain
Copy link
Contributor

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

No branches or pull requests

5 participants