Request errors #126
Merged
Request errors #126
Conversation
This file contains 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
wadetandy
force-pushed
the
request_errors
branch
from
76112f6
to
39818e9
Compare
|
@richmolj Would like to get your feedback on the approach before I drive across the finish line. The current implementation would replace the typecasting & writability checks currently in |
Instead of having the message change dramatically depending on whether an AttributeError is a missing attribute or an access error, we subclass and create a message based on the subclass.
This is the first phase of work to allow us to provide better & more actionable errors to API users who might not also be developers of the API itself. Previously when trying to write unwritable attributes & relationships, on typecast problems, or trying to write an attribute that doesn't exist, the request would blow up with a 500 and an unhelpful error message. The request validator takes on most of the work of transforming and validating an incoming request for processing all while rescuing any of the above errors and turning them into helpful error messages. Note that this is only currently intended to process write payloads, but will be fully extendable to validate query parameters for read requests too. This doesn't actually integrate the validator into the code path, as I want to get feedback on the design before doing that work in case anything needs to change. This will take over some of the normalization responsibilities which currently live in `Graphiti::Util::Persistence`. This also adds a new "payload_path" key to the meta payload for the deserialized/normalized request payload, which allows consumers to know which path within the request json structure they are dealing with at any given time. Here it is used for more useful error messages.
wadetandy
force-pushed
the
request_errors
branch
from
39818e9
to
43caaca
Compare
wadetandy
force-pushed
the
request_errors
branch
from
43caaca
to
6ad47e6
Compare
|
@richmolj This should be good to go. Going to follow up with changes to graphiti_errors later this weekend/early next week so that we can render nice errors back to API consumers. |
wadetandy
force-pushed
the
request_errors
branch
from
6ad47e6
to
6da3770
Compare
The `Graphiti::Base` module was a legacy from jsonapi_suite that was mixed into the `Graphiti::Runner` and `Graphiti` modules to provide them both with functionality. Turns out we weren't using the latter case anymore so there's no reason for this to remain a separate concern.
wadetandy
force-pushed
the
request_errors
branch
from
6da3770
to
bc1d903
Compare
Calling `.find()`, `.build()`, or `.all()` on a resource will now run validations on the incoming params hash and raise an error if the payload is incorrect. It will also be easy to integrate url request params for read requests, as we will just need to add the error handling to the validator and they will correctly propogate.
wadetandy
force-pushed
the
request_errors
branch
from
bc1d903
to
892a0f6
Compare
richmolj
approved these changes
Apr 30, 2019
| @@ -180,12 +164,8 @@ def process_has_many(relationships, caller_model) | |||
| iterate(except: [:polymorphic_belongs_to, :belongs_to]) do |x| | |||
| yield x | |||
|
|
|||
| if x[:sideload].writable? | |||
There was a problem hiding this comment.
because this check happens upstream in the validator right?
There was a problem hiding this comment.
As far as I can tell this can't happen in a way that doesn't go through the runner which does the validator check. Let me know if this isn't true for any reason you're aware of.
wadetandy
added a commit
to wadetandy/graphiti
that referenced
this pull request
Apr 30, 2019
wadetandy
force-pushed
the
request_errors
branch
from
7f9d307
to
e789708
Compare
|
Pushed updates. Let's get graphiti_errors#5 merged and released before this is merged in. |
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.
This is the first phase of work to allow us to provide better & more
actionable errors to API users who might not also be developers of the
API itself. Previously when trying to write unwritable attributes &
relationships, on typecast problems, or trying to write an attribute
that doesn't exist, the request would blow up with a 500 and an
unhelpful error message. The request validator takes on most of the
work of transforming and validating an incoming request for processing
all while rescuing any of the above errors and turning them into helpful
error messages. Note that this is only currently intended to process
write payloads, but will be fully extendable to validate query
parameters for read requests too.
This also adds a new "payload_path" key to the meta payload for the
deserialized/normalized request payload, which allows consumers to know
which path within the request json structure they are dealing with at
any given time. Here it is used for more useful error messages.