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

Store exceptions to be reported later #9601

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 34 additions & 0 deletions common/lib/dependabot/exception_capturer.rb
@@ -0,0 +1,34 @@
# typed: strong
# frozen_string_literal: true

require "sorbet-runtime"

module Dependabot
module ExceptionCapturer
extend T::Sig

# An array of captured exceptions stored for later retrieval
@captured_exceptions = T.let([], T::Array[StandardError])

sig { params(error: StandardError).void }
def self.capture_exception(error:)
@captured_exceptions << error
end

sig { params(block: T.proc.params(error: StandardError).void).void }
def self.handle_captured_exceptions(&block)
@captured_exceptions.each(&block)
clear_captured_exceptions
end

sig { returns(T::Array[StandardError]) }
def self.captured_exceptions
@captured_exceptions
end

sig { void }
def self.clear_captured_exceptions
@captured_exceptions = []
end
end
end
Expand Up @@ -887,6 +887,7 @@ def package_manager

sig { params(method: String, err: StandardError).void }
def suppress_error(method, err)
Dependabot::ExceptionCapturer.capture_exception(error: err)
Dependabot.logger.error("Error while generating #{method}: #{err.message}")
Dependabot.logger.error(err.backtrace&.join("\n"))
end
Expand Down
Expand Up @@ -5,6 +5,7 @@
require "spec_helper"
require "dependabot/dependency"
require "dependabot/dependency_file"
require "dependabot/exception_capturer"
require "dependabot/pull_request_creator/message_builder"

RSpec.describe Dependabot::PullRequestCreator::MessageBuilder do
Expand Down
8 changes: 7 additions & 1 deletion updater/bin/update_files.rb
Expand Up @@ -10,6 +10,7 @@
require "dependabot/service"
require "dependabot/setup"
require "dependabot/update_files_command"
require "dependabot/exception_capturer"
require "debug" if ENV["DEBUG"]

flamegraph = ENV.fetch("FLAMEGRAPH", nil)
Expand All @@ -31,7 +32,12 @@ class UpdaterKilledError < StandardError; end
Dependabot::Environment.job_id,
Dependabot::Environment.job_token
)
Dependabot::Service.new(client: api_client).capture_exception(error: error, tags: tags)
service = Dependabot::Service.new(client: api_client)

Dependabot::ExceptionCapturer.handle_captured_exceptions do |exception|
service.capture_exception(error: exception, tags: tags)
end
service.capture_exception(error: error, tags: tags)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to put this down below 👇 We're eating the exception in the MessageBuilder so it won't reach this code in the trap.

What might be simpler would be give the ExceptionCapturer the service here (outside of the trap), and have it log to the API immediately when capture_exception is called.

exit
end

Expand Down
4 changes: 4 additions & 0 deletions updater/lib/dependabot/update_files_command.rb
Expand Up @@ -51,6 +51,10 @@ def perform_job
# successfully processed unless it actually raises.
service.mark_job_as_processed(dependency_snapshot.base_commit_sha)
end
ensure
Dependabot::ExceptionCapturer.handle_captured_exceptions do |exception|
service.capture_exception(error: exception, job: job)
end
end

private
Expand Down
4 changes: 4 additions & 0 deletions updater/spec/dependabot/update_files_command_spec.rb
Expand Up @@ -2,7 +2,10 @@
# frozen_string_literal: true

require "spec_helper"
require "dependabot/exception_capturer"
require "dependabot/update_files_command"
require "dependabot/file_parsers"
require "dependabot/bundler"
require "tmpdir"

RSpec.describe Dependabot::UpdateFilesCommand do
Expand All @@ -24,6 +27,7 @@
let(:job_id) { "123123" }

before do
Dependabot::FileParsers.register("bundler", ::Dependabot::Bundler::FileParser)
allow(Dependabot::Service).to receive(:new).and_return(service)
allow(Dependabot::Environment).to receive(:job_id).and_return(job_id)
allow(Dependabot::Environment).to receive(:job_token).and_return("mock_token")
Expand Down