Skip to content

Commit

Permalink
Avoid reporting the same Bundler install error twice (#3033)
Browse files Browse the repository at this point in the history
### Motivation

If bundle install fails and one of the gems ends up not being present, then `Bundler.setup` is guaranteed to fail with `Bundler::GemNotFound`, but that's expected. The root cause of the problem is the fact that installation failed.

Let's not set `setup_error` in that particular case to avoid reporting the same issue twice to telemetry.

### Implementation

Started not setting `setup_error` if there was an `install_error` and the error during setup matches the expected class.
  • Loading branch information
vinistock authored Jan 9, 2025
1 parent d40ccce commit 41fceb9
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions exe/ruby-lsp-launcher
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ rescue Errno::ECHILD
# In theory, the child process can finish before we even get to the wait call, but that is not an error
end

error_path = File.join(".ruby-lsp", "install_error")

install_error = if File.exist?(error_path)
Marshal.load(File.read(error_path))
end

begin
bundle_env_path = File.join(".ruby-lsp", "bundle_env")
# We can't require `bundler/setup` because that file prematurely exits the process if setup fails. However, we can't
Expand All @@ -67,21 +73,21 @@ begin
rescue StandardError => e
# If installing gems failed for any reason, we don't want to exit the process prematurely. We can still provide most
# features in a degraded mode. We simply save the error so that we can report to the user that certain gems might be
# missing, but we respect the LSP life cycle
setup_error = e
$stderr.puts("Failed to set up composed Bundle\n#{e.full_message}")
# missing, but we respect the LSP life cycle.
#
# If an install error occurred and one of the gems is not installed, Bundler.setup is guaranteed to fail with
# `Bundler::GemNotFound`. We don't set `setup_error` here because that would essentially report the same problem twice
# to telemetry, which is not useful
unless install_error && e.is_a?(Bundler::GemNotFound)
setup_error = e
$stderr.puts("Failed to set up composed Bundle\n#{e.full_message}")
end

# If Bundler.setup fails, we need to restore the original $LOAD_PATH so that we can still require the Ruby LSP server
# in degraded mode
$LOAD_PATH.unshift(File.expand_path("../lib", __dir__))
end

error_path = File.join(".ruby-lsp", "install_error")

install_error = if File.exist?(error_path)
Marshal.load(File.read(error_path))
end

# Now that the bundle is set up, we can begin actually launching the server. Note that `Bundler.setup` will have already
# configured the load path using the version of the Ruby LSP present in the composed bundle. Do not push any Ruby LSP
# paths into the load path manually or we may end up requiring the wrong version of the gem
Expand Down

0 comments on commit 41fceb9

Please sign in to comment.