graphiti-api / graphiti

Stylish Graph APIs
https://www.graphiti.dev/
MIT License
974 stars 139 forks source link

Handle malformed parameters with a 400 instead of a 500 #386

Closed sideshowbandana closed 2 years ago

sideshowbandana commented 3 years ago

When processing an invalid request that is missing the following headers:

'Content-Type': 'application/json'

and has the following body:

{'data': 'relationships'}

results in:

TypeError (no implicit conversion of Symbol into Integer):

activesupport (6.1.4.1) lib/active_support/core_ext/object/try.rb:15:in `[]'
activesupport (6.1.4.1) lib/active_support/core_ext/object/try.rb:15:in `public_send'
activesupport (6.1.4.1) lib/active_support/core_ext/object/try.rb:15:in `try'
graphiti (1.3.3) lib/graphiti/resource/links.rb:75:in `block in
allow_request?'

Which responds with a 500 status.

Instead the behavior should respond with a 400.

sideshowbandana commented 3 years ago

@richmolj what do you think of this PR?

richmolj commented 3 years ago

I think the issue is this breaks backwards-compatibility. I suggest throwing errors in this case and users can register those errors if they want 400. Also there is a codepath for this...you see we hit a RequestValidator for writes but it is a pass-through for reads...so making a true validator class is the way to go I think

sideshowbandana commented 2 years ago

Sounds good, thanks for the suggestion!