Skip to content

Standardised error codes in exceptions? #105

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

Closed
cportele opened this issue Apr 3, 2018 · 10 comments
Closed

Standardised error codes in exceptions? #105

cportele opened this issue Apr 3, 2018 · 10 comments
Assignees
Labels
Part 1: Core Issue related to Part 1 - Core

Comments

@cportele
Copy link
Member

cportele commented Apr 3, 2018

WFS 2.0 defined exception codes (http://docs.opengeospatial.org/is/09-025r2/09-025r2.html#35) and mapped them to HTTP status codes (http://docs.opengeospatial.org/is/09-025r2/09-025r2.html#411).

Is there any value in specifying error codes for exception responses also in WFS 3.0 or should we simply require that http status codes are used correctly?

@cportele cportele added the Part 1: Core Issue related to Part 1 - Core label Apr 3, 2018
@cmheazel
Copy link
Contributor

cmheazel commented Apr 3, 2018

Locking down the error codes in the OWS standards is one reason why we had to create the OWS Security standard. Error codes will change (expand) over time. Unless there are WFS 3.0 specific behaviors that we have to specify, defer to the HTTP standards.

@cholmes
Copy link
Member

cholmes commented Apr 3, 2018

I do think there's a ton of value in providing guidance for the http status codes. I don't know that they need to be 'locked down' or specified strongly. But I think it could be good to at least have recommendations or something in the user's guide. I think it'd also be good for a test engine to test them (could make it so that getting them wrong doesn't make one 'fail' but raises a strong recommendation).

The problem I see is that most geospatial developers don't really know about what error codes to use. Sure, they could look at the http standard and try to figure out how all their responses map to it. But if we could do some intellectual work to give a solid set of recommendations of good error codes for common use cases I think that would help a lot of people implement the spec more easily.

Thanks for bringing this up @cportele - I've actually been meaning to raise this, as I find it to be one of the big holes in the spec, it says 'use good error codes' but doesn't give any great example of what they meant. I hadn't seen that list - it does look pretty good.

@cmheazel
Copy link
Contributor

cmheazel commented Apr 3, 2018

I could live with that. As long as we focus on those codes which may be of interest to WFS developers, and we don't preclude other codes, then I think we are good.

@cportele
Copy link
Member Author

cportele commented Apr 4, 2018

OK, so if we want to list the common responses per operation then we would add something like the following to each resource?

/collections/{name}/items:

  • 200: a successful request
  • 304: if a entity tag was provided and the resource has not been changed since the previous request
  • 400: if limit is not an integer or not between minimum and maximum; if bbox does not have 4 (or 6) numbers or they do not form a bounding box; if time is not a valid time stamp or time period
  • 404: if name is not a collection
  • 405: the request method is not supported (i.e. for a "pure" Core WFS it was not GET - or HEAD or OPTIONS; by the way: should we also require or recommend support for HEAD and OPTIONS?)
  • 406: the accept header did not support any of the media types supported by the server for the resource
  • 500: an internal error occurred in the server

Everything except the 400 and 404 applies to all the resources in the Core, so maybe have a general section that is referenced from every resource (we can add the statement that other status codes are OK, too, and could add security related status codes there later). Then add more specific guidance in the section of each resource, which typically will be 400 or 404 in the Core.

Should we try to still include this in the first release or just keep the issue open for now?

@pvretano - Any opinion on this?

cportele added a commit that referenced this issue Apr 4, 2018
@jvanulde
Copy link
Contributor

jvanulde commented Apr 5, 2018

Maybe 200, 400, and 404 will cover most cases?

@cholmes
Copy link
Member

cholmes commented Apr 5, 2018

Not sure if anyone has cycles for it, but I do think it'd be good to get in to the first release. I do fear people not implementing because there's just not enough guidance in the core.

It'd also be good to include them in the OpenAPI documents.

@pvretano
Copy link
Contributor

pvretano commented Apr 5, 2018

@cportele

  1. I think the error codes, as you have laid them out, are fine.
  2. HEAD? Yes, since you get it almost for free anyway.
  3. OPTIONS? I want to say yes because it is very useful for clients and not that difficult to implement
  4. I think getting the error codes into the first release would be good. Is there time for that?
  5. We can add HEAD and OPTIONS after TB14 (... and make sure we try to test HEAD and OPTIONS during TB14).

@cportele
Copy link
Member Author

cportele commented Apr 6, 2018

OK, I will give it a try today.

And I will open a new issue for HEAD and OPTIONS. We need OPTIONS also for CORS preflight requests.

@cportele
Copy link
Member Author

cportele commented Apr 6, 2018

I have created new issue #115 for HEAD/OPTIONS.

I have also created a pull request for the status codes.

See https://rawgit.com/opengeospatial/WFS_FES/issue-105/docs/17-069.html#http_status_codes for the additional section. For each resource I have added a section on error situations. For example https://rawgit.com/opengeospatial/WFS_FES/issue-105/docs/17-069.html#_error_situations_6.

If I do not hear any objections I will merge the pull request tomorrow before the draft.1 release.

@pvretano
Copy link
Contributor

pvretano commented Apr 6, 2018

@cportele no objection from me so merge when ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Part 1: Core Issue related to Part 1 - Core
Projects
Development

No branches or pull requests

5 participants