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-&gt;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-&gt;expandItem(&#039;type&#039;, &#039;some_value&#039;, Array) (Line: 139)
Drupal\jsonapi\Normalizer\FilterNormalizer-&gt;expand(Array, Array) (Line: 96)
Drupal\jsonapi\Normalizer\FilterNormalizer-&gt;denormalize(Array, &#039;Drupal\jsonapi\Query\Filter&#039;, NULL, Array) (Line: 73)
Drupal\jsonapi\Routing\JsonApiParamEnhancer-&gt;enhance(Array, Object) (Line: 259)
Drupal\Core\Routing\Router-&gt;applyRouteEnhancers(Array, Object) (Line: 130)
Drupal\Core\Routing\Router-&gt;matchRequest(Object) (Line: 90)
Drupal\Core\Routing\AccessAwareRouter-&gt;matchRequest(Object) (Line: 114)
Symfony\Component\HttpKernel\EventListener\RouterListener-&gt;onKernelRequest(Object, &#039;kernel.request&#039;, Object)
call_user_func(Array, Object, &#039;kernel.request&#039;, Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher-&gt;dispatch(&#039;kernel.request&#039;, Object) (Line: 127)
Symfony\Component\HttpKernel\HttpKernel-&gt;handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel-&gt;handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session-&gt;handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle-&gt;handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache-&gt;pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache-&gt;handle(Object, 1, 1) (Line: 40)
Drupal\jsonapi\StackMiddleware\FormatSetter-&gt;handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware-&gt;handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware-&gt;handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel-&gt;handle(Object, 1, 1) (Line: 664)
Drupal\Core\DrupalKernel-&gt;handle(Object) (Line: 19)
CommentFileSizeAuthor
#8 2968891-8.patch1.58 KBwim leers
#8 interdiff.txt831 byteswim leers
#7 2968891-7.patch803 byteswim leers

Comments

mstef created an issue. See original summary.

mstef’s picture

Category: Bug report » Feature request
Issue summary: View changes

Changing to a feature request since it's not a bug. Feel free to close if you don't agree with me.

gabesullice’s picture

Title: Fatal error for incorrectly formatted filters: TypeError: Argument 2 passed to Drupal\jsonapi\Normalizer\FilterNormalizer::expandItem() must be of the type array » Return 4xx client error for malformed filters, not 500 unhandled exception
Category: Feature request » Bug report
Issue summary: View changes
Issue tags: +API-First Initiative

Great catch @mstef. Thanks!

I agree that we should be doing a better job here.

mstef’s picture

Adding 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.

wim leers’s picture

Issue summary: View changes
Issue tags: +DX (Developer Experience)
wim leers’s picture

Title: Return 4xx client error for malformed filters, not 500 unhandled exception » Allow extreme shorthand filtering: ?filter[promote]=1
Category: Bug report » Feature request
Parent issue: » #2842148: The path for JSON API to "super stability"

https://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?

wim leers’s picture

Status: Active » Needs review
StatusFileSize
new803 bytes

Test coverage, this should fail.

wim leers’s picture

StatusFileSize
new831 bytes
new1.58 KB

And the actual logic changes.

The last submitted patch, 7: 2968891-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

gabesullice’s picture

Status: Needs review » Reviewed & tested by the community

Very nice. I like it.

mstef’s picture

Great 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.

gabesullice’s picture

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.

Yeah, let's make another ticket for it :)

wim leers’s picture

  • Wim Leers committed aa3cd16 on 8.x-1.x
    Issue #2968891 by Wim Leers, mstef, gabesullice: Allow extreme shorthand...
  • Wim Leers committed 23c1434 on 8.x-2.x
    Issue #2968891 by Wim Leers, mstef, gabesullice: Allow extreme shorthand...
gabesullice’s picture

wim leers’s picture

👌 — thank you! And sorry, I should've thought of that!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.