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

Add error reporting as per section 7.1 from RFC-8040 #17

Closed
pdumais opened this issue Dec 21, 2022 · 3 comments
Closed

Add error reporting as per section 7.1 from RFC-8040 #17

pdumais opened this issue Dec 21, 2022 · 3 comments

Comments

@pdumais
Copy link
Contributor

pdumais commented Dec 21, 2022

When returning an error with
// Option #2 - enhanced but still an 401 error return fmt.Errorf("Bad ACL %w", fc.UnauthorizedError)
It would be useful if restconf could create a payload according to the model of ietf-restconf:errors

{
  "ietf-restconf:errors": {
    "error": [
      {
        "error-type": "protocol",
        "error-tag": "lock-denied",
        "error-message": "Lock failed; lock already held"
      }
    ]
  }
}
@dhubler
Copy link
Collaborator

dhubler commented Dec 21, 2022

dhubler added a commit that referenced this issue Jan 2, 2023
@dhubler dhubler closed this as completed Jan 2, 2023
@sebastien-guay
Copy link

sebastien-guay commented Feb 24, 2023

Hi @dhubler, I see that the response body is well formatted:

{"ietf-sztp-bootstrap-server:output":}{"ietf-restconf:errors":{"error":[{"error-type":"protocol","error-tag":"access-denied","error-path":"ietf-sztp-bootstrap-server:get-bootstrapping-data","error-message":"Bad ACL not authorized"}

But the response code is always 200 OK and it should be 401.

We are completely uploaded and fineTLSv1.3 (IN), TLS handshake, Newsession Ticket (4): Connection state changed (MAX_CONCURRENT_STREAMS == 250)! HTTP/2 200 access-control-allow-headers: origin, content-type, accept access-control-allow-methods: GET, POST, PUT, OPTIONS, DELETE, PATCH access-control-allow-origin: * content-type: application/json content-length: 236 date: Fri, 24 Feb 2023 19:34:07 GMT {"ietf-sztp-bootstrap-server:output":}{"ietf-restconf:errors":{"error":[{"error-type":"protocol","error-tag":"access-denied","error-path":"ietf-sztp-bootstrap-server:get-bootstrapping-data","error-message":"Bad ACL not authorized"} ]}}

Is it the right way to return an error:

return nil, fmt.Errorf("Bad ACL %w", fc.UnauthorizedError)

@dhubler
Copy link
Collaborator

dhubler commented Mar 3, 2023

It should be a 401 and it does look like you are wrapping the error correctly.

If you are sure you have haven't added more error wrapping beyond the above and running the latest code then there might be an issue.

BTW: The related code is here:
https://github.com/freeconf/restconf/blob/master/util.go#L71
https://github.com/freeconf/yang/blob/master/fc/err.go#L31
It would help if you can setup a debugger and step thru why the status code might be getting lost.

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

No branches or pull requests

3 participants