Bug 31022 - 400/404 actually returns 500
Summary: 400/404 actually returns 500
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-06-23 07:59 UTC by Jonathan Druart
Modified: 2023-06-18 11:59 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2022-06-23 07:59:56 UTC
Not sure if I am missing something, but I don't manage to get a 400 with the syntax we are using all around our REST API code.

  return $c->render(
    status  => 400,
    openapi => { error => "this is an error" }
  );

I am getting a 500 with, in Koha logs:

  [{"message":"Missing property.","path":"\/body\/errors"}] 

So yes, it is working with:

  return $c->render(
    status  => 400,
    openapi => { errors => [ {message => 'this is an error'}] }
  );

But it's not at all what we are doing in our REST API module.
Comment 1 Martin Renvoize 2022-06-23 09:00:24 UTC
We narrowed this one down.. it's the patrons search (GET) we're looking at and it appears we're missing our local '400' definition in the spec file /paths/patrons.yaml
Comment 2 Jonathan Druart 2022-06-23 09:01:26 UTC
Shouldn't we add the different errors for each route?
Comment 3 Martin Renvoize 2022-06-23 09:02:29 UTC
I do however find it really weird still that you can define more than one definition for the same response status on the endpoint...

i.e. the internal 400 definitions for the validator software and then our own one for our own errors.

I would have expected our definition to take precedence over the internal one and thus need it to be a superset of the built-in one.

Interesting.
Comment 4 Tomás Cohen Arazi 2022-07-06 20:12:56 UTC
(In reply to Martin Renvoize from comment #3)
> I do however find it really weird still that you can define more than one
> definition for the same response status on the endpoint...
> 
> i.e. the internal 400 definitions for the validator software and then our
> own one for our own errors.
> 
> I would have expected our definition to take precedence over the internal
> one and thus need it to be a superset of the built-in one.
> 
> Interesting.

Doesn't our definition just overload the default/internal one? It used to return a 500 because it wasn't considered a valid response in the spec, which I find correct.
Comment 5 Tomás Cohen Arazi 2022-07-06 20:26:49 UTC
Ok, tested myself, this is some Mojolicious::Plugin::OpenAPI bug.

Reported it: https://github.com/jhthorsen/mojolicious-plugin-openapi/issues/237
Comment 6 Katrin Fischer 2023-06-18 11:59:53 UTC
(In reply to Tomás Cohen Arazi from comment #5)
> Ok, tested myself, this is some Mojolicious::Plugin::OpenAPI bug.
> 
> Reported it:
> https://github.com/jhthorsen/mojolicious-plugin-openapi/issues/237

I see that the bug has been closed - anything we can do here now?