Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider revising the structure of Exception responses #180

Closed
ghobona opened this issue Sep 30, 2020 · 14 comments
Closed

Consider revising the structure of Exception responses #180

ghobona opened this issue Sep 30, 2020 · 14 comments
Assignees
Labels
2020-09 Sprint Collections Applicable to Collections (consider to use Part 2 instead)

Comments

@ghobona
Copy link
Contributor

ghobona commented Sep 30, 2020

There was a suggestion to consider revising the structure of Exception responses, so that they could be as informative as those from OWS Common.

Currently the structure is as shown at https://github.com/opengeospatial/oapi_common/blob/master/core/openapi/schemas/exception.yaml

One suggestion is to have an array of descriptions.

The 'code' property is not well defined and it appears to duplicate the role of the HTTP status code.

Suggestion that there would be a recommendation to use the JSON Schema for exception responses, and the Guide would provide more information.

@pomakis
Copy link

pomakis commented Sep 30, 2020

I think this is a "Common-Part 1" issue (as apposed to "Common-Part 2"). It's certainly not specific to collections.

@pomakis
Copy link

pomakis commented Sep 30, 2020

I propose the following schema for the body of OGC API responses with an HTTP 4xx or 5xx status code:

{
  "type": "object",
  "required": [ "type", "reason" ],
  "properties": {
    "type": {
      "enum": [ "OgcApiExceptionReport" ]
    },
    "reason" {
      "type": "array",
      "items": {
        "type": "object",
        "required": [ "description" ],
        "properties": { "type": "string" }
      }
      "minItems": 1
    }
  }
}

The first (and required) element of the "reason" array should be the primary or most general reason for the exception. If the client wants a single, concise string to report to the user, this is the one to report. Further elements in the array can be used to provide drill-down reasons.

Examples:

{
  "type": "OgcApiExceptionReport",
  "reason": [
    { "description": "Illegal value specified for parameter \"bbox\"" },
    { "description": "Cannot parse string \"df\" as a floating-point value" }
  ]
}
{
  "type": "OgcApiExceptionReport",
  "reason": [
    { "description": "Illegal value specified for parameter \"geodata\"" },
    { "description": "No such collection \"coastlines\"" }
  ]
}

The reason I propose an array of objects with a single "description" string is to allow for future or vendor-specific fields such as (e.g.):

{
  "type": "OgcApiExceptionReport",
  "reason": [
    {
      "description": "Illegal value specified for parameter \"bbox\"",
      "traceInfo": {
        "filename": "ogcApi_common.c",
        "function": "OgcApi_GetRequestBbox()",
        "line": 465
      }
    },
    {
      "description": "Cannot parse string \"df\" as a floating-point value",
      "traceInfo": {
        "filename": "cw_str.c",
        "function": "CwStr_ParseDouble()",
        "line": 2561
      }
    }
  ]
}

This exception-report format should be a recommendation rather than a requirement since for some servers and/or error cases it may not be possible or feasible to generate this form of output. A client application can easily sniff out whether or not it has received such an exception report by checking that the Content Type of the response is "application/json" and that there's a top-level "type": "OgcApiExceptionReport" property.

This, of course, describes JSON responses only. Presumably format negotiation should be honoured where feasible when generating the body of an HTTP 4xx or 5xx response. For example, if the client requested HTML (through the Accept header, an "f" parameter or whatever), then an HTML exception report should be returned if feasible.

@jerstlouis
Copy link
Member

jerstlouis commented Sep 30, 2020

@pomakis

A few comments.

Shouldn't the trace info support multiple stack frames? A single frame might often be quite useless without the higher up context.

Should reason be plural since it's an array?
How about details instead of reason? It's hard for a server to know the true reason why an error occurs, but it can provide some additional details about it :)

I still think it's a good thing to specify the HTTP code in there... This way if you save only the JSON error log and save it to inspect later, or file it in a bug report, you're sure the HTTP status code is preserved.

@pomakis
Copy link

pomakis commented Sep 30, 2020

I made "reason" singular rather than plural because I see it as a single reason with possibly multiple levels of detail. But I'm equally fine with "details".

I'm not sure what you mean by "multiple stack frames". An error typically happens for a single specific reason.

I'm okay with having a (redundant) "httpStatus" property. I personally see it as pointless, but I won't object if others see value in it.

@ghobona
Copy link
Contributor Author

ghobona commented Sep 30, 2020

Thanks @pomakis . Label changed to Common-Part 1.

@jerstlouis
Copy link
Member

jerstlouis commented Sep 30, 2020

@pomakis I much prefer details.

Re: trace info I mean being able to list a call stack with multiple levels of sourceFile:lineNo so you know what function/line invoked CwStr_ParseDouble().

I see value in httpStatus in the JSON.

@pomakis
Copy link

pomakis commented Sep 30, 2020

Re: trace info I mean being able to list a call stack with multiple levels of sourceFile:lineNo so you know what function/line invoked CwStr_ParseDouble().

Ah, that was only an example of a future or vendor-specific extension. I'm not proposing adding anything like a "traceInfo" property at this point.

@pomakis
Copy link

pomakis commented Sep 30, 2020

So now (with a couple of other schema corrections) we have:

{
  "type": "object",
  "required": [ "type", "details" ],
  "properties": {
    "type": {
      "enum": [ "OgcApiExceptionReport" ]
    },
    "httpStatus": {
      "type": "integer"
    },
    "details": {
      "type": "array",
      "items": {
        "type": "object",
        "required": [ "description" ],
        "properties": {
          "description": {
            "type": "string"
          }
        }
      },
      "minItems": 1
    }
  }
}

@pomakis
Copy link

pomakis commented Dec 8, 2020

I have upgraded CubeWerx's test implementation at

https://test.cubewerx.com/cubewerx/cubeserv/demo/ogcapi/Daraa

to report exceptions using this schema.

@ghobona ghobona transferred this issue from opengeospatial/OGC-API-Sprint-September-2020 Dec 11, 2020
@cmheazel
Copy link
Contributor

This issue was resolved in issue #73

@joanma747
Copy link
Contributor

joanma747 commented Feb 22, 2021

The resolution in #73 applies: "API-Common Part 1 - the exception schema has been updated to conform with RFC 7807. This schema will be updated to conform with the revised version of RFC 7807 once it is finalized." Will be also applied to part 2 collections. In telco 2021-02-22

@joanma747 joanma747 added Collections Applicable to Collections (consider to use Part 2 instead) and removed Close labels Feb 22, 2021
@cmheazel
Copy link
Contributor

API-Common Part 2 is now in-line with Part 1.

@aaime
Copy link

aaime commented Oct 26, 2021

A note that the current definition of exception:
https://github.com/opengeospatial/ogcapi-common/blob/master/core/openapi/schemas/exception.yaml
is inconsistent with the Features API one:
http://schemas.opengis.net/ogcapi/features/part1/1.0/openapi/schemas/exception.yaml

So I guess that implementations of OGC API Features should either:

  • Not declare conformance with OGC API Common
  • Report all fields, with eventually duplicate information (e.g., same value for code and type, and some of the same information in description and title/detail

@cmheazel
Copy link
Contributor

See issue #73
The resolution was to align with IETF RFC 7807. Since RFC 7807 is being revised, we accept that we will have to update both Common and Features once the RFC 7807 (or its replacement) is finalized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2020-09 Sprint Collections Applicable to Collections (consider to use Part 2 instead)
Projects
None yet
Development

No branches or pull requests

6 participants