Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

bundle exec rescues 'SignalException' and aborts #6090

Closed
dekellum opened this issue Oct 11, 2017 · 4 comments · Fixed by #6092
Closed

bundle exec rescues 'SignalException' and aborts #6090

dekellum opened this issue Oct 11, 2017 · 4 comments · Fixed by #6092

Comments

@dekellum
Copy link
Contributor

This was first reported in puma/puma#1438. As of puma 3.10.0, SIGTERM handling was changed (in puma/puma#1337) to properly report the termination-by-SIGTERM status back to the parent process. This is accomplished by re-raising a SignalException. Unfortunately when puma is run via bundle exec puma and bundle exec uses CLI::Exec#kernel_load, the SignalException is rescued by bundler, logged to stderr, and the process is aborted via Kernel::abort (exit status 1). I believe that minimally, kernel_load should not be rescuing SignalException.

An interesting possible workaround for this is that a bundle binstub produced wrapper does not include the same error handling as CLI::Exec#kernel_load and thus does not exhibit the same problem.

This can be reproduced via a simple, public, puma test app:

https://github.com/dekellum/lionfish

A Gemfile and Gemfile.lock is checked in.

My testing bundler version: 1.15.1

Below is log output what happens when the puma process is signaled with SIGTERM for a graceful restart. Note the "bundle: failed to load command" output and stack trace from bundle exec Here exit status 1 is interpreted as a failure by systemd. From the original puma report (see above) this similarly confuses Heroku.

Oct 11 09:51:34 klein systemd[826]: Stopping Lionfish (Puma) HTTP Server...
Oct 11 09:51:34 klein bundle[25925]: - Gracefully stopping, waiting for requests to finish
Oct 11 09:51:34 klein bundle[25925]: === puma shutdown: 2017-10-11 09:51:34 -0700 ===
Oct 11 09:51:34 klein bundle[25925]: - Goodbye!
Oct 11 09:51:34 klein bundle[25925]: bundler: failed to load command: puma (/home/david/.gem/ruby/2.3.0/bin/puma)
Oct 11 09:51:34 klein bundle[25925]: SignalException: SIGTERM
Oct 11 09:51:34 klein bundle[25925]:   /home/david/.gem/ruby/2.3.0/gems/puma-3.10.0/lib/puma/launcher.rb:397:in `block in setup_signals'
Oct 11 09:51:34 klein bundle[25925]:   /home/david/.gem/ruby/2.3.0/gems/puma-3.10.0/lib/puma/single.rb:106:in `join'
Oct 11 09:51:34 klein bundle[25925]:   /home/david/.gem/ruby/2.3.0/gems/puma-3.10.0/lib/puma/single.rb:106:in `run'
Oct 11 09:51:34 klein bundle[25925]:   /home/david/.gem/ruby/2.3.0/gems/puma-3.10.0/lib/puma/launcher.rb:183:in `run'
Oct 11 09:51:34 klein bundle[25925]:   /home/david/.gem/ruby/2.3.0/gems/puma-3.10.0/lib/puma/cli.rb:77:in `run'
Oct 11 09:51:34 klein bundle[25925]:   /home/david/.gem/ruby/2.3.0/gems/puma-3.10.0/bin/puma:10:in `<top (required)>'
Oct 11 09:51:34 klein bundle[25925]:   /home/david/.gem/ruby/2.3.0/bin/puma:22:in `load'
Oct 11 09:51:34 klein bundle[25925]:   /home/david/.gem/ruby/2.3.0/bin/puma:22:in `<top (required)>'
Oct 11 09:51:34 klein systemd[826]: lionfish.service: Main process exited, code=exited, status=1/FAILURE
Oct 11 09:51:34 klein systemd[826]: Stopped Lionfish (Puma) HTTP Server.
Oct 11 09:51:34 klein systemd[826]: lionfish.service: Unit entered failed state.
Oct 11 09:51:34 klein systemd[826]: lionfish.service: Failed with result 'exit-code'.
Oct 11 09:51:34 klein systemd[826]: Started Lionfish (Puma) HTTP Server.
dekellum added a commit to dekellum/bundler that referenced this issue Oct 11, 2017
Make `bundle exec` via Kernel.load handle exceptions just like a
bundler binstub: Kernel.load is called and any exceptions raised are
allowed to bubble up and be handled in the default way by the ruby
interpreter.

github: fixes rubygems#6090
dekellum added a commit to dekellum/bundler that referenced this issue Oct 11, 2017
This allows SignalException to be handled gracefully by the ruby
interpreter.

github: fixes rubygems#6090
dekellum added a commit to dekellum/bundler that referenced this issue Oct 14, 2017
This allows SignalException to be handled gracefully by the ruby
interpreter.

(rebased on to master with CI Fix, 6084)

github: fixes rubygems#6090
@colby-swandale
Copy link
Member

\cc @segiddins thoughts?

@segiddins
Copy link
Member

Seems like a reasonable request, and they submitted a PR for it

@dekellum
Copy link
Contributor Author

Just in case the type label somehow sets expectations, for example backporting to a stable branch. This is a bug report IMO, not a feature request, thus I'm surprised about the feature-request label.

@dekellum
Copy link
Contributor Author

Note, in course of PR and testing, this is another viable workaround to avoid this issue:

bundle config disable_exec_load true

In other words, if bundle exec does a Kernel::exec (the older behavior) instead of Kernel::load, then the exec'd child process has no additional error handling getting in the way, and SignalException is handled by the ruby interpreter correctly.

bundlerbot added a commit that referenced this issue Oct 18, 2017
…ddins

Avoid bundle exec rescue of SignalException

Avoid rescue of SignalException in kernel_load and with_friendly_errors This allows SignalException to be handled gracefully by the ruby interpreter.

This supersedes #6091.

### What was the end-user problem that led to this PR?

Fixes #6090

### What was your diagnosis of the problem?

CLI::Exec#kernel_load should not rescue the SignalException. Neither should with_friendly_errors, so it unwinds all the way to the ruby interpreter.

### What is your fix for the problem, implemented in this PR?

Two rescue and re-raises specific to SignalException.

### Why did you choose this fix out of the possible options?

My first naive attempt was #6091, where I had missed the outer rescue in with_friendly_errors, all of the other special friendly error behavior, and the spec dependencies. While I still suspect that kernel_load and friendly_errors might be rescuing/handling more than they should (for comparison: binstub doesn't do this) this is a much more minimal, safer, and easier fix to #6090.
segiddins pushed a commit that referenced this issue Oct 30, 2017
…ddins

Avoid bundle exec rescue of SignalException

Avoid rescue of SignalException in kernel_load and with_friendly_errors This allows SignalException to be handled gracefully by the ruby interpreter.

This supersedes #6091.

### What was the end-user problem that led to this PR?

Fixes #6090

### What was your diagnosis of the problem?

CLI::Exec#kernel_load should not rescue the SignalException. Neither should with_friendly_errors, so it unwinds all the way to the ruby interpreter.

### What is your fix for the problem, implemented in this PR?

Two rescue and re-raises specific to SignalException.

### Why did you choose this fix out of the possible options?

My first naive attempt was #6091, where I had missed the outer rescue in with_friendly_errors, all of the other special friendly error behavior, and the spec dependencies. While I still suspect that kernel_load and friendly_errors might be rescuing/handling more than they should (for comparison: binstub doesn't do this) this is a much more minimal, safer, and easier fix to #6090.

(cherry picked from commit 6d0b74c)
hone added a commit to heroku/heroku-buildpack-ruby that referenced this issue Jan 15, 2018
…ndle exec.

See rubygems/bundler#6090. This stops
SignalException from being handled by Bundler when using `bundle exec`
and instead by Ruby. As of Puma 3.10.0, Puma changed the SIGTERM
hanlding to be passed back to the parent process, `bundle exec` in this
case.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Mar 14, 2018
## 1.16.1 (2017-12-12)

Bugfixes:

  - avoid hanging on complex resolver errors ([#6114](rubygems/bundler#6114), @halfbyte)
  - avoid an error when running `bundle update --group` ([#6156](rubygems/bundler#6156), @mattbrictson)
  - ensure the resolver prefers non-pre-release gems when possible ([#6181](rubygems/bundler#6181), @greysteil)
  - include bundler's gemspec in the built gem ([#6165](rubygems/bundler#6165), @dr-itz)
  - ensure locally installed specs are not overriden by those in remote sources during dependency resolution ([#6072](rubygems/bundler#6072), @indirect)
  - ensure custom gemfiles are respected in generated binstubs (@pftg)
  - fail gracefully when loading a bundler-generated binstub when `bin/bundle` was not generated by bundler ([#6149](rubygems/bundler#6149), @hsbt)
  - allow `bundle init` to be run even when a parent directory contains a gemfile ([#6205](rubygems/bundler#6205), @colby-swandale)

## 1.16.0 (2017-10-31)

Bugfixes:

  - avoid new RubyGems warning about unsafe YAML loading (to keep output consistent) (@segiddins)
  - load digest subclasses in a thread-safe manner (@segiddins, @colby-swandale)
  - avoid unusued variable warnings under ruby 2.5 (@amatsuda)
  - fix printing the same message twice in verbose mode ([#6028](rubygems/bundler#6028), @akhramov)
  - allow `SignalException`s to bubble up to the interpreter during `bundle exec` ([#6090](rubygems/bundler#6090), @dekellum)
  - avoid activating stdlib digest under Ruby 2.5 (@segiddins)
  - prioritise explicitly requested gems in dependency resolution sort order (@segiddins)
  - reduce memory usage during dependency resolution ([#6114](rubygems/bundler#6114), @greysteil)
  - ensure that the default bundler gem is not accidentally activated on ruby 2.5 when using local git overrides (@segiddins)

## 1.16.0.pre.3 (2017-10-04)

Features:

  - the output from `bundle env` includes more information, particularly both the compiled & loaded versions of OpenSSL (@indirect)

Bugfixes:

  - fix a bug where installing on FreeBSD would accidentally raise an error (#6013, @olleolleolle)
  - fix a regression in 1.16 where pre-release gems could accidentally be resolved even when the gemfile contained no pre-release requirements (@greysteil)
  - bundler will avoid making unnecessary network requests to fetch dependency data, fixing a regression introduced in 1.16 (@segiddins)
  - the outdated bundler version message is disabled by default until the message has been fine-tuned (#6004, @segiddins)

## 1.16.0.pre.2 (2017-09-06)

Bugfixes:

  - handle when a connection is missing a socket when warning about OpenSSL version (@greysteil)
  - the description for the `rake release` task now reflects `$RUBYGEMS_HOST` (@wadetandy)
  - fix a bug where `bundle update` would regress transitive dependencies (@greysteil)

## 1.16.0.pre.1 (2017-09-04)

Features:

  - allow using non-branch symbolic refs in a git source (#4845, @segiddins)
  - allow absolute paths in the `cache path` setting (#5627, @mal)
  - gems created via `bundle gem` with rspec have `--require spec_helper` in their `.rspec` file (@koic)
  - `bundle env` includes `Gem.ruby` and the `bundle` binstub shebang when they don't match (#5616, @segiddins)
  - allow passing gem names to `bundle pristine` (@segiddins)
  - `bundle version` and `bundle env` include the commit and build date for the bundler gem (#5049, @segiddins)
  - add the `--shebang` option to `bundle binstubs` (#4070, @segiddins, @zorbash)
  - gemfiles are `eval`ed one fewer time when running `bundle install` (#4952, #3096, #4417, @segiddins)
  - the `fileutils` gem is now vendored so different versions of the gem can be activated (@segiddins)
  - speed up no-op installations (#5842, @segiddins)
  - default to keeping the lockfile in the default gem template (@deivid-rodriguez)
  - add a special bundler binstub that ensures the correct version of bundler is activated (#5876, @segiddins)
  - speed up dependency resolution and ensure that all resolvable gemfiles can be installed (@segiddins, @greysteil)
  - add a `bundle list` command that prints the gems in use (#4754, @colby-swandale)
  - allow adding credentials to a gem source during deployment when `allow_deployment_source_credential_changes` is set (@adrian-gomez)
  - making an outdated (and insecure) TLS connection to rubygems.org will print a warning (@segiddins)

Bugfixes:

  - allow configuring a mirror fallback timeout without a trailing slash (#4830, @segiddins)
  - fix handling of mirrors for file: urls that contain upper-case characters (@segiddins)
  - list the correct gem host for `rake release` when `allowed_push_host` has been set (@mdeering)
  - ensure `Bundler.original_env` preserves all env keys that bundler sets (#5700, @segiddins)
  - ensure `bundle pristine` removes files added to a git gem (@segiddins)
  - load plugin files from path gems before gem installation (#5429, @segiddins)
  - ensure gems containing manpages are properly set up (#5730, @segiddins)
  - avoid fetching remote specs when all effected gems are in groups that are not being installed (@segiddins)
  - allow `BUNDLE_GEMFILE` to be a relative path (#5712, @gxespino)
  - show a more helpful error message when a gem fails to install due to a corrupted lockfile (#5846, @segiddins)
  - add a process lock to allow multiple concurrent `bundle install`s (#5851, @stefansedich)
  - ensure that specifications always return an array for `#extensions` (@greysteil)
  - print a helpful error message when using a gem in the Gemfile with an empty name (@colby-swandale)
  - ensure that all gemfiles are included in `bundle env` (@segiddins)
  - use ssl client cert and ca cert settings from gem configuration as fallbacks (@stan3)
  - avoid global namespace pollution when loading gems (#5958, @shyouhei)
  - avoid running a complete re-resolve on `bundle update --bundler` (@segiddins)
  - allow `bundle binstubs --standalone` to work without `path` being set (@colby-swandale)
  - fix support for bundle paths that include jars or wars on jruby (#5975, @torcido)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants