From 41fceb9f8371b4b80f15ae40fa46e976573680d3 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Thu, 9 Jan 2025 13:50:47 -0500 Subject: [PATCH] Avoid reporting the same Bundler install error twice (#3033) ### 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. --- exe/ruby-lsp-launcher | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/exe/ruby-lsp-launcher b/exe/ruby-lsp-launcher index a89095c71..c6ead1d8f 100755 --- a/exe/ruby-lsp-launcher +++ b/exe/ruby-lsp-launcher @@ -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 @@ -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