Refactor error handling #8486
Refactor error handling #8486
Conversation
deivid-rodriguez
force-pushed
the
deivid-rodriguez/refactor-errors
branch
from
13fb2d3
to
eb1584d
Compare
deivid-rodriguez
force-pushed
the
deivid-rodriguez/refactor-errors
branch
from
eb1584d
to
7fd4fc7
Compare
| raise unless error_details | ||
|
|
||
| puts " => handled error whilst fetching dependencies: #{error_details.fetch(:"error-type")} " \ | ||
| "#{error_details.fetch(:"error-detail")}" | ||
|
|
||
| [] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
That's neat, I think it'd be a cool follow up to this PR!
It's not straightforward to know where new errors need to be added, so that they are properly passed back to the server.
Also, error handling is duplicated in dry-run.rb script and updater commands.
This PR tries to improve both of the above things.