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

Reconsider status codes for DRU immutable and duplicate process responses #328

Closed
fmigneault opened this issue May 30, 2023 · 6 comments
Closed

Comments

@fmigneault
Copy link
Contributor

fmigneault commented May 30, 2023

/req/deploy-replace-undeploy/deploy/response-duplicate and /req/deploy-replace-undeploy/deploy/response-immutable both use the same "error" code 303 See Other.

This makes it impossible for a Web Client interacting with the API to properly handle the problem, because it could be any of the two errors. Futhermore, many clients automatically handle 3xx codes by silently/automatically following the returned redirect Location, causing the final response toward an existing process to be 200 OK. This leads to potential confusion by the Web Client user (ie: the user deploys/deletes a process, gets an 200 OK, but the obtained process description is wrong/still defined). Also, the Location of the "other" process could point toward a resource that is unaccessible, due to security restrictions or limitations for the identified user performing the original request. This instead can cause Authentication 401 or 403 responses after following the Location, which is even less descriptive about the cause of the problem.

In the case of a duplicate process, 409 Conflict would be more relevant:

409 Conflict
The request could not be completed due to a conflict with the current state of the resource.
[...] Whenever a resource conflict would be caused by fulfilling the request. Duplicate entries and deleting root objects when cascade-delete is not supported are a couple of examples.

In the case of an immutable process, the 403 Forbidden operation would make more sense:

403 Forbidden
The server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the server wishes to make public why the request has not been fulfilled, it SHOULD describe the reason for the refusal in the entity.

On top of being more relevant codes to illustrate the error that occured, 4xx are actually classified as "Client Error".
Codes in the 3xx group are for redirections (aka the equivalent of 2xx but with an extra Location), which is misleading to indicate errors.

@fmigneault
Copy link
Contributor Author

@pvretano @jerstlouis @gfenoy Thoughts about this?

@gfenoy
Copy link
Contributor

gfenoy commented May 30, 2023

@fmigneault thanks a lot for pointing this out.

I am in favour of this modifications and would be please to work on integrating them in this PR.

gfenoy added a commit to GeoLabs/ogcapi-processes that referenced this issue May 30, 2023
@gfenoy
Copy link
Contributor

gfenoy commented May 30, 2023

@fmigneault I have integrated the modifications here: GeoLabs@dbac653#diff-6126b7f18b1eafbd6697a4be54ec5ec85dbbde6c71422940ff7d5bba582e95d6.

Please confirm that it fixes the issue.

@fmigneault
Copy link
Contributor Author

@gfenoy
Thanks! That would work for me. Only the .adoc would need to be adjusted to reflect those changes.

gfenoy added a commit to GeoLabs/ogcapi-processes that referenced this issue May 30, 2023
@gfenoy
Copy link
Contributor

gfenoy commented May 30, 2023

@fmigneault thanks your feedback.

This time it should be fine.

@pvretano
Copy link
Contributor

21-AUG-2023: Closed by #327.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants