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.

Sign up for GitHub

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

Jump to bottom

Refactor error handling #8486

Merged
merged 5 commits into from
Nov 29, 2023
Merged

Refactor error handling #8486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5723990
Remove unnecessary condition
deivid-rodriguez Nov 29, 2023
23404f8
Extract fetcher error mapping to a method
deivid-rodriguez Nov 29, 2023
8fc4f80
Extract parser error mapping to a method
deivid-rodriguez Nov 29, 2023
7948091
Extract updater error mapping to a method
deivid-rodriguez Nov 29, 2023
7fd4fc7
Reuse common error methods in dry-run script
deivid-rodriguez Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 15 additions & 89 deletions bin/dry-run.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,101 +402,26 @@ def fetch_files(fetcher)
end
end
rescue StandardError => e
error_details = handle_dependabot_error(error: e)
error_details = Dependabot.fetcher_error_details(e)
raise unless error_details

puts " => handled error whilst fetching dependencies: #{error_details.fetch(:"error-type")} " \
"#{error_details.fetch(:"error-detail")}"

[]
Comment on lines +406 to 411
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why we cannot raise when there are error details? Is that because we only throw when there are unknown exceptions?

Also, rather than puts, is there any reason not to use the Logger with error level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed the same strategy already used for fetching files. Same for using puts, I just did what we were already doing in other error handling blocks.

end

# rubocop:disable Metrics/MethodLength
def handle_dependabot_error(error:)
case error
when Dependabot::DependencyFileNotResolvable
{
"error-type": "dependency_file_not_resolvable",
"error-detail": { message: error.message }
}
when Dependabot::DependencyFileNotEvaluatable
{
"error-type": "dependency_file_not_evaluatable",
"error-detail": { message: error.message }
}
when Dependabot::BranchNotFound
{
"error-type": "branch_not_found",
"error-detail": { "branch-name": error.branch_name }
}
when Dependabot::DirectoryNotFound
{
"error-type": "directory_not_found",
"error-detail": { "directory-name": error.directory_name }
}
when Dependabot::DependencyFileNotParseable
{
"error-type": "dependency_file_not_parseable",
"error-detail": {
message: error.message,
"file-path": error.file_path
}
}
when Dependabot::DependencyFileNotFound
{
"error-type": "dependency_file_not_found",
"error-detail": { "file-path": error.file_path }
}
when Dependabot::PathDependenciesNotReachable
{
"error-type": "path_dependencies_not_reachable",
"error-detail": { dependencies: error.dependencies }
}
when Dependabot::GitDependenciesNotReachable
{
"error-type": "git_dependencies_not_reachable",
"error-detail": { "dependency-urls": error.dependency_urls }
}
when Dependabot::GitDependencyReferenceNotFound
{
"error-type": "git_dependency_reference_not_found",
"error-detail": { dependency: error.dependency }
}
when Dependabot::PrivateSourceAuthenticationFailure
{
"error-type": "private_source_authentication_failure",
"error-detail": { source: error.source }
}
when Dependabot::PrivateSourceTimedOut
{
"error-type": "private_source_timed_out",
"error-detail": { source: error.source }
}
when Dependabot::PrivateSourceCertificateFailure
{
"error-type": "private_source_certificate_failure",
"error-detail": { source: error.source }
}
when Dependabot::MissingEnvironmentVariable
{
"error-type": "missing_environment_variable",
"error-detail": {
"environment-variable": error.environment_variable
}
}
when Dependabot::GoModulePathMismatch
{
"error-type": "go_module_path_mismatch",
"error-detail": {
"declared-path": error.declared_path,
"discovered-path": error.discovered_path,
"go-mod": error.go_mod
}
}
else
raise
end
def parse_dependencies(parser)
cached_read("dependencies") { parser.parse }
rescue StandardError => e
error_details = Dependabot.parser_error_details(e)
raise unless error_details

puts " => handled error whilst parsing dependencies: #{error_details.fetch(:"error-type")} " \
"#{error_details.fetch(:"error-detail")}"

[]
end
# rubocop:enable Metrics/MethodLength

def log_conflicting_dependencies(conflicting_dependencies)
return unless conflicting_dependencies.any?
Expand Down Expand Up @@ -568,7 +493,7 @@ def log_conflicting_dependencies(conflicting_dependencies)
reject_external_code: $options[:reject_external_code]
)

dependencies = cached_read("dependencies") { parser.parse }
dependencies = parse_dependencies(parser)

if $options[:dependency_names].nil?
dependencies.select!(&:top_level?)
Expand Down Expand Up @@ -810,7 +735,8 @@ def security_fix?(dependency)
puts "--commit--\n#{msg.commit_message}\n--/commit--"
end
rescue StandardError => e
error_details = handle_dependabot_error(error: e)
error_details = Dependabot.updater_error_details(e)
raise unless error_details

puts " => handled error whilst updating #{dep.name}: #{error_details.fetch(:"error-type")} " \
"#{error_details.fetch(:"error-detail")}"
Expand Down
204 changes: 204 additions & 0 deletions common/lib/dependabot/errors.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,210 @@
require "dependabot/utils"

module Dependabot
# rubocop:disable Metrics/MethodLength
def self.fetcher_error_details(error)
case error
when Dependabot::ToolVersionNotSupported
{
"error-type": "tool_version_not_supported",
"error-detail": {
"tool-name": error.tool_name,
"detected-version": error.detected_version,
"supported-versions": error.supported_versions
}
}
when Dependabot::BranchNotFound
Comment on lines +11 to +20
Copy link
Contributor

@yeikel yeikel Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think that it would be ideal if each of these exceptions produced these via their constructors and exposed a dedicated method to fetch this parseable error message. If we had that, we would not need this mapping at all as it would be something like error.fetcher_error_details or similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's neat, I think it'd be a cool follow up to this PR!

{
"error-type": "branch_not_found",
"error-detail": { "branch-name": error.branch_name }
}
when Dependabot::DirectoryNotFound
{
"error-type": "directory_not_found",
"error-detail": { "directory-name": error.directory_name }
}
when Dependabot::RepoNotFound
# This happens if the repo gets removed after a job gets kicked off.
# This also happens when a configured personal access token is not authz'd to fetch files from the job repo.
{
"error-type": "job_repo_not_found",
"error-detail": { message: error.message }
}
when Dependabot::DependencyFileNotParseable
{
"error-type": "dependency_file_not_parseable",
"error-detail": {
message: error.message,
"file-path": error.file_path
}
}
when Dependabot::DependencyFileNotFound
{
"error-type": "dependency_file_not_found",
"error-detail": { "file-path": error.file_path }
}
when Dependabot::OutOfDisk
{
"error-type": "out_of_disk",
"error-detail": {}
}
when Dependabot::PathDependenciesNotReachable
{
"error-type": "path_dependencies_not_reachable",
"error-detail": { dependencies: error.dependencies }
}
when Octokit::Unauthorized
{ "error-type": "octokit_unauthorized" }
when Octokit::ServerError
# If we get a 500 from GitHub there's very little we can do about it,
# and responsibility for fixing it is on them, not us. As a result we
# quietly log these as errors
{ "error-type": "server_error" }
when *Octokit::RATE_LIMITED_ERRORS
# If we get a rate-limited error we let dependabot-api handle the
# retry by re-enqueing the update job after the reset
{
"error-type": "octokit_rate_limited",
"error-detail": {
"rate-limit-reset": error.response_headers["X-RateLimit-Reset"]
}
}
end
end

def self.parser_error_details(error)
case error
when Dependabot::DependencyFileNotEvaluatable
{
"error-type": "dependency_file_not_evaluatable",
"error-detail": { message: error.message }
}
when Dependabot::DependencyFileNotResolvable
{
"error-type": "dependency_file_not_resolvable",
"error-detail": { message: error.message }
}
when Dependabot::BranchNotFound
{
"error-type": "branch_not_found",
"error-detail": { "branch-name": error.branch_name }
}
when Dependabot::DependencyFileNotParseable
{
"error-type": "dependency_file_not_parseable",
"error-detail": {
message: error.message,
"file-path": error.file_path
}
}
when Dependabot::DependencyFileNotFound
{
"error-type": "dependency_file_not_found",
"error-detail": { "file-path": error.file_path }
}
when Dependabot::PathDependenciesNotReachable
{
"error-type": "path_dependencies_not_reachable",
"error-detail": { dependencies: error.dependencies }
}
when Dependabot::PrivateSourceAuthenticationFailure
{
"error-type": "private_source_authentication_failure",
"error-detail": { source: error.source }
}
when Dependabot::GitDependenciesNotReachable
{
"error-type": "git_dependencies_not_reachable",
"error-detail": { "dependency-urls": error.dependency_urls }
}
when Dependabot::NotImplemented
{
"error-type": "not_implemented",
"error-detail": {
message: error.message
}
}
when Octokit::ServerError
# If we get a 500 from GitHub there's very little we can do about it,
# and responsibility for fixing it is on them, not us. As a result we
# quietly log these as errors
{ "error-type": "server_error" }
end
end

def self.updater_error_details(error)
case error
when Dependabot::DependencyFileNotResolvable
{
"error-type": "dependency_file_not_resolvable",
"error-detail": { message: error.message }
}
when Dependabot::DependencyFileNotEvaluatable
{
"error-type": "dependency_file_not_evaluatable",
"error-detail": { message: error.message }
}
when Dependabot::GitDependenciesNotReachable
{
"error-type": "git_dependencies_not_reachable",
"error-detail": { "dependency-urls": error.dependency_urls }
}
when Dependabot::GitDependencyReferenceNotFound
{
"error-type": "git_dependency_reference_not_found",
"error-detail": { dependency: error.dependency }
}
when Dependabot::PrivateSourceAuthenticationFailure
{
"error-type": "private_source_authentication_failure",
"error-detail": { source: error.source }
}
when Dependabot::PrivateSourceTimedOut
{
"error-type": "private_source_timed_out",
"error-detail": { source: error.source }
}
when Dependabot::PrivateSourceCertificateFailure
{
"error-type": "private_source_certificate_failure",
"error-detail": { source: error.source }
}
when Dependabot::MissingEnvironmentVariable
{
"error-type": "missing_environment_variable",
"error-detail": {
"environment-variable": error.environment_variable
}
}
when Dependabot::GoModulePathMismatch
{
"error-type": "go_module_path_mismatch",
"error-detail": {
"declared-path": error.declared_path,
"discovered-path": error.discovered_path,
"go-mod": error.go_mod
}
}
when Dependabot::NotImplemented
{
"error-type": "not_implemented",
"error-detail": {
message: error.message
}
}
when *Octokit::RATE_LIMITED_ERRORS
# If we get a rate-limited error we let dependabot-api handle the
# retry by re-enqueing the update job after the reset
{
"error-type": "octokit_rate_limited",
"error-detail": {
"rate-limit-reset": error.response_headers["X-RateLimit-Reset"]
}
}
end
end
# rubocop:enable Metrics/MethodLength

class DependabotError < StandardError
extend T::Sig

Expand Down
Loading
Loading