This is not really a bug because the it's only caused when filter parameters in the endpoint are incorrectly formatted, but it's my opinion that it should be handled more gracefully than the 500 fatal error response you get now.
To reproduce this error, incorrectly format a filter parameter, for example:
/jsonapi/entity_type?sort=created&filter[some_field]=some_value
The correct way to make the call is:
/jsonapi/entity_type?sort=created&filter[some_field][value]=some_value
The error is:
The website encountered an unexpected error. Please try again later.<br /><em class="placeholder">TypeError</em>: Argument 2 passed to Drupal\jsonapi\Normalizer\FilterNormalizer::expandItem() must be of the type array, string given, called in /var/www/web/modules/contrib/jsonapi/src/Normalizer/FilterNormalizer.php on line 139 in <em class="placeholder">Drupal\jsonapi\Normalizer\FilterNormalizer->expandItem()</em> (line <em class="placeholder">164</em> of <em class="placeholder">modules/contrib/jsonapi/src/Normalizer/FilterNormalizer.php</em>). <pre class="backtrace">Drupal\jsonapi\Normalizer\FilterNormalizer->expandItem('type', 'some_value', Array) (Line: 139)
Drupal\jsonapi\Normalizer\FilterNormalizer->expand(Array, Array) (Line: 96)
Drupal\jsonapi\Normalizer\FilterNormalizer->denormalize(Array, 'Drupal\jsonapi\Query\Filter', NULL, Array) (Line: 73)
Drupal\jsonapi\Routing\JsonApiParamEnhancer->enhance(Array, Object) (Line: 259)
Drupal\Core\Routing\Router->applyRouteEnhancers(Array, Object) (Line: 130)
Drupal\Core\Routing\Router->matchRequest(Object) (Line: 90)
Drupal\Core\Routing\AccessAwareRouter->matchRequest(Object) (Line: 114)
Symfony\Component\HttpKernel\EventListener\RouterListener->onKernelRequest(Object, 'kernel.request', Object)
call_user_func(Array, Object, 'kernel.request', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.request', Object) (Line: 127)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 40)
Drupal\jsonapi\StackMiddleware\FormatSetter->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 664)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 2968891-8.patch | 1.58 KB | wim leers |
Comments
Comment #2
mstef commentedChanging to a feature request since it's not a bug. Feel free to close if you don't agree with me.
Comment #3
gabesulliceGreat catch @mstef. Thanks!
I agree that we should be doing a better job here.
Comment #4
mstef commentedAdding a field name that does not exist in a filter or sort does return a 4xx code but it also returns a full HTML render of the page; rather than JSON. It would be nice if it matched the unprocessable entity responses.
I looked in to the original issue. Not sure yet how to handle the exception that early in the response.
Comment #5
wim leersComment #6
wim leershttps://www.drupal.org/docs/8/modules/json-api/filtering#shortcuts does not mention this, but I'd swear that you could omit the
[value]bit. I must be mistaken.In any case, this is a perfect example of one of the things I think we should do as part of #2842148: The path for JSON API to "super stability".
I dug in, and found that
\Drupal\jsonapi\Normalizer\EntityConditionNormalizer::validate()is in fact validating the presence/absence of this key, and\Drupal\jsonapi\Normalizer\EntityConditionNormalizer::denormalize()allows that key to be optionally not set! But this ironically runs at a later time, and hence doesn't end up making a difference.So rather than throwing the appropriate exception, I think this should … work?
Comment #7
wim leersTest coverage, this should fail.
Comment #8
wim leersAnd the actual logic changes.
Comment #10
gabesulliceVery nice. I like it.
Comment #11
mstef commentedGreat job. Works very well.
Requesting fields that do not exist in filters or sort still return a 400 but with an HTML response (rather than JSON). Not sure if that's acceptable or should just be moved in to a different ticket than this one.
Comment #12
gabesulliceYeah, let's make another ticket for it :)
Comment #13
mstef commentedOkay - tracking here #2969398: When using JSON API Extras' ability to change the base path, 4xx errors returned for using an invalid field in filter or sort returns HTML response rather than JSON
Comment #14
wim leersThanks for creating #2969398: When using JSON API Extras' ability to change the base path, 4xx errors returned for using an invalid field in filter or sort returns HTML response rather than JSON!
Since we have an RTBC here from a maintainer and somebody from our userbase tested it, committing this :)
Comment #16
gabesulliceDocs updated: https://www.drupal.org/node/2943641/revisions/view/10881668/10952779
Comment #17
wim leers👌 — thank you! And sorry, I should've thought of that!