Raise specific error classes for 4xx and 5xx errors#839
Open
omkarmoghe wants to merge 2 commits intohttprb:mainfrom
Open
Raise specific error classes for 4xx and 5xx errors#839omkarmoghe wants to merge 2 commits intohttprb:mainfrom
omkarmoghe wants to merge 2 commits intohttprb:mainfrom
Conversation
Member
|
Why are the tests failing? I can't even tell |
Contributor
|
@tarcieri Tests are all passing but exit non-zero because code coverage dropped below 100%. Also, RuboCop also found linting errors, Steep type check failed, and Mutant failed. Assuming those issues are fixed, I’m curious what you think about this proposal, in general? |
Contributor
Author
|
Yeah, happy to fix the Rubocop linter issues if the maintainers like the overall idea. It's fairly trivial (method/case statement too long). Happy to add more test coverage for each branch too. |
Member
|
This seems fine to me as a general direction |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context/problem
When using the
http.rbgem in production code, I often find myself writing boilerplate code to wrap theStatusErrorthrown by the gem and raise a more specific error based on the response code.This is often in some client class or shared HTTP classes that can be used across the codebase. This works, but has a few issues:
responseinstance var to be useful. You end up re-implementingStatusErrorin the app, or you inherit directly from the gem class.Proposal (this PR)
I would like to propose that the HTTP gem enumerate the roughly ~40 or so 4xx and 5xx status codes. This would provide a clean interface that allows callers to catch errors at varying levels of granularity, while being fully backwards compatible.
To achieve this, I'd like to propose 2 subclasses of
StatusError:ClientError, an entry point for all 4xx errorsServerError, an entry point for all 5xx errorsBecause all specific error classes are ultimately instances of
StatusError, this should not break any existingrescueblocks. Users can opt-in to more granular errors as they see fit.Example migration
Existing code
After change
Existing code still works as-is
Would love any feedback or thoughts, thanks.