jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
53 stars 41 forks source link

404 not handled properly in v3 #179

Closed zaucker closed 3 years ago

zaucker commented 4 years ago

While the v2 example shown below runs successfully (all tests pass) in the v3 example the expected 404 error is "converted" to a 500 error with message No responses rules defined for */*.

v2:

use Mojo::Base -strict;
use Mojo::File 'path';
use Test::Mojo;
use Test::More;
use Mojolicious::Lite;

plugin OpenAPI => { url => 'data://main/404.json', schema => 'v2' };

my $t = Test::Mojo->new;
$t->get_ok('/v1/unknownRoute')->status_is(404)
  ->json_is('/errors/0/message', 'Not Found.');
done_testing;
__DATA__
@@ 404.json
{
  "swagger": "2.0",
  "info": {
    "title": "404 Test",
    "version": "1.0.0"
  },
  "basePath": "/v1",
  "schemes": ["http"],
  "consumes": ["application/json"],
  "produces": ["application/json"],
  "paths": {
  },
  "definitions": {
    "Error": {
      "required": ["code", "message"],
      "properties": {
        "code": {"type": "integer", "format": "int32"},
        "message": {"type": "string"}
      }
    }
  }
}

v3:

use Mojo::Base -strict;
use Mojo::File 'path';
use Test::Mojo;
use Test::More;
use Mojolicious::Lite;

plugin OpenAPI => { url =>  'data://main/404.json', schema => 'v3' };

my $t = Test::Mojo->new;
$t->get_ok('/v1/unknownRoute')->status_is(404)
  ->json_is('/errors/0/message', 'Not Found.');
done_testing;
__DATA__
@@ 404.json
{
  "openapi": "3.0.0",
  "info": {
    "title": "404 Test",
    "version": "1.0.0"
  },
  "paths": {
  },
  "servers": [
      {
         "url": "/v1"
      }
  ],
  "components": {
    "schemas": {
      "Error": {
        "required": [ "code", "message" ],
        "properties": {
          "code": { "format": "int32", "type": "integer" },
          "message": { "type": "string" }
        }
      }
    }
  }
}

The "conversion" happens in JSON::Validator::OpenAPI::Mojolicious::validate_response() with

  if ($self->version eq '3') {
    my $accept = $self->_negotiate_accept_header($c, $res_schema);
    return JSON::Validator::E('/' => "No responses rules defined for $accept.")
      unless $res_schema = $res_schema->{content}{$accept};
    $c->stash(openapi_negotiated_content_type => $accept);
  }

Would this mean that the v3 schema is missing some kind of default specification? It seems wrong to me that a 404 is not reported as such to the user, but "hidden" behind a (in principal correct) missing spec error hiding the real problem. Or is this intentional? I couldn't find anything on that in the OpenAPI specs. BTW, the problem is the same if the specs contain paths specifications and an unknown route is being called.

zaucker commented 4 years ago

Changing the above if-statement to if ($self->version eq '3' and $status != 404) would solve the problem for this example, but that is probably not the right thing to do ...

jhthorsen commented 3 years ago

I'm not sure if my solution was less hackish than yours, but this is now fixed. Will be available on CPAN soon.