Make errors more consistent and explanatory

[gamemaker1]

Bad Requests

Describe the bug
When I made a create entity request with two required fields missing, I got the following error:

HTTP/1.1 400 
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: close
Content-Type: application/json
Date: Wed, 20 Oct 2021 10:33:51 GMT
Expires: 0
Pragma: no-cache
Transfer-Encoding: chunked
Vary: Origin, Access-Control-Request-Method, Access-Control-Request-Headers
X-Content-Type-Options: nosniff
X-Frame-Options: DENY
X-XSS-Protection: 1; mode=block

{
    "ets": 1634726031652,
    "id": "open-saber.registry.invite",
    "params": {
        "err": "",
        "errmsg": "Validation Exception : #/Teacher : #/Teacher: 2 schema violations found",
        "msgid": "53cefdb9-288f-4cc5-9f9d-d108837afbee",
        "resmsgid": "",
        "status": "UNSUCCESSFUL"
    },
    "responseCode": "OK",
    "ver": "1.0"
}

Expected behavior
Since the status code is 400, responseCode should be Bad Request, not OK. Also, it would be helpful to the client if the error explains why the error occurred and how it could be resolved. Here is an example of the error message a developer/client application would like to receive for a bad request:

HTTP/1.1 400 
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: close
Content-Type: application/json
Date: Wed, 20 Oct 2021 10:33:51 GMT
Expires: 0

X-Server: http://rg:8081/
X-Request-Id: 53cefdb9-288f-4cc5-9f9d-d108837afbee

{
  "error": {
    "code": 400,
	"status": "Bad Request"
    "errors": [
      {
        "operation": "registry.invite",
        "location": "request.body.email",
        "message": "`email` is a mandatory field in the `Teacher` schema. Please provide the email in the request body alongside the other attributes and try the request again.",
        "reason": "badRequest"
      },
      {
        "operation": "registry.invite",
        "location": "request.body.subject",
        "message": "`subject` is an enum - it's value can only be one of 'Math', 'Hindi', 'English', 'History', 'Geography', 'Physics', 'Chemistry', 'Biology' in the `Teacher` schema. Please provide a valid subject in the request body alongside the other attributes and try the request again.",
        "reason": "badRequest"
      }
    ],
    "message": "Two schema violations detected. Please make the requested changes and retry the request."
  }
}

It would be more consistent if a similar error is returned when I make a request to send a claim for attestation, but make an error while sending the request body. Currently, I get the following error:

HTTP/1.1 404 
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: keep-alive
Content-Type: application/json
Date: Wed, 20 Oct 2021 11:23:55 GMT
Expires: 0
Keep-Alive: timeout=60
Pragma: no-cache
Set-Cookie: JSESSIONID=B817F10950DBB4AB426CD927EB14C1EF; Path=/; HttpOnly
Transfer-Encoding: chunked
Vary: Origin, Access-Control-Request-Method, Access-Control-Request-Headers
X-Content-Type-Options: nosniff
X-Frame-Options: DENY
X-XSS-Protection: 1; mode=block

{
    "ets": 1634729035682,
    "id": "open-saber.registry.update",
    "params": {
        "err": "",
        "errmsg": "Validation Exception : #/Student/school : #/Student/school: expected type: String, found: JSONObject",
        "msgid": "f973e0d6-7a76-4d8d-8616-d7b051373987",
        "resmsgid": "",
        "status": "UNSUCCESSFUL"
    },
    "responseCode": "OK",
    "ver": "1.0"
}

Since the error is regarding a bad request, the returned HTTP status code should be 400, not 404. The responseCode in the request body should be Bad Request, not OK. An example error message that would be great (similar to the one above):

HTTP/1.1 400 
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: close
Content-Type: application/json
Date: Wed, 20 Oct 2021 10:33:51 GMT
Expires: 0

X-Server: http://rg:8081/
X-Request-Id: f973e0d6-7a76-4d8d-8616-d7b051373987

{
  "error": {
    "code": 400,
	"status": "Bad Request"
    "errors": [
      {
        "operation": "registry.update-claim",
        "location": "request.body",
        "message": "`school` is of type `String` in the `Teacher` schema. Please provide only the value for `school` in the request body as a JSON `String` (found `JSONObject` instead) and try the request again.",
        "reason": "badRequest"
      }
    ],
    "message": "One schema violation detected. Please make the requested change and retry the request."
  }
}

Forbidden Requests

Describe the bug
When I made a get entity request with the access token for another entity, I got the following error:

HTTP/1.1 403 
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: keep-alive
Content-Length: 0
Date: Wed, 20 Oct 2021 10:49:13 GMT
Expires: 0
Keep-Alive: timeout=60
Pragma: no-cache
Set-Cookie: JSESSIONID=937462B8331D4B7ABB4BA9DE8DCBF199; Path=/; HttpOnly
Vary: Origin, Access-Control-Request-Method, Access-Control-Request-Headers
X-Content-Type-Options: nosniff
X-Frame-Options: DENY
X-XSS-Protection: 1; mode=block

Expected behavior
It would be helpful to the client if there was an error object returned that explains why the error occurred and how it could be resolved. Here is an example of the error message a client application/developer would like to receive for a forbidden request:

HTTP/1.1 403
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: close
Content-Type: application/json
Date: Wed, 20 Oct 2021 10:33:51 GMT
Expires: 0

X-Server: http://rg:8081/
X-Request-Id: 67ckfddn-288f-998s-00xk-d82s8FidSbee

{
  "error": {
    "code": 403,
	"status": "Forbidden"
    "errors": [
      {
        "operation": "registry.retrieve",
        "location": "request.headers.authorization",
        "message": "You do not have sufficient permissions to perform this operation.",
        "reason": "forbidden"
      }
    ],
    "message": "You are not permitted to perform this operation."
  }
}

The same error should be returned when no access token is provided. Currently, it returns a redirect to the keycloak login page:

HTTP/1.1 302 
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: keep-alive
Content-Length: 0
Date: Wed, 20 Oct 2021 10:55:27 GMT
Expires: 0
Keep-Alive: timeout=60
Location: http://kc:8080/auth/realms/sunbird-rc/protocol/openid-connect/auth?response_type=code&client_id=registry-frontend&redirect_uri=http%3A%2F%2Flocalhost%3A8081%2Fapi%2Fv1%2FStudent%2F1-cccea3dd-47af-494b-acf6-442f5bcf586c&state=0b032fae-39f3-4f7a-a761-9cddda52089b&login=true&scope=openid
Pragma: no-cache
Set-Cookie: OAuth_Token_Request_State=0b032fae-39f3-4f7a-a761-9cddda52089b; Path=/; HttpOnly
X-Content-Type-Options: nosniff
X-Frame-Options: DENY
X-XSS-Protection: 1; mode=block

Even if I follow the URL and authenticate as the user, it takes me to a whitelabel error page:

Since this is an API call (/api/v1/...), it will most probably be a developer or their application making a request, and not a user in their browser. A developer/client application will want a 403 error as a response instead of a 200 code and an HTML login page from keycloak.

Expired/Invalid Token

Describe the bug
When I make a request with an expired access token, I get the following error:

HTTP/1.1 401 
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: keep-alive
Content-Length: 0
Date: Wed, 20 Oct 2021 11:05:25 GMT
Expires: 0
Keep-Alive: timeout=60
Pragma: no-cache
WWW-Authenticate: Bearer realm="sunbird-rc", error="invalid_token", error_description="Token is not active", Bearer realm="sunbird-rc", error="invalid_token", error_description="Token is not active"
X-Content-Type-Options: nosniff
X-Frame-Options: DENY
X-XSS-Protection: 1; mode=block

Expected behavior
It is not a good practice to return errors in headers. cURL does not even show response headers or a status code by default - a developer would be puzzled as to what happened if they didn't use certain flags to show the response headers in cURL. Here is an example of the error message an API client/developer would like to receive for a request with invalid/expired credentials:

HTTP/1.1 401
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Connection: close
Content-Type: application/json
Date: Wed, 20 Oct 2021 10:33:51 GMT
Expires: 0

X-Server: http://rg:8081/
X-Request-Id: 8x8ksaw0c-7hk2-1lxu-cls5-83ld8Fi7glwb

{
  "error": {
    "code": 403,
	"status": "Forbidden"
    "errors": [
      {
        "operation": "registry.retrieve",
        "location": "request.headers.authorization",
        "message": "Your access token is invalid or expired. Please get a new access token and retry this request with it.",
        "reason": "invalidCredentials"
      }
    ],
    "message": "Your access token is invalid or expired."
  }
}

[gamemaker1]

Here is a good flowchart showing which HTTP response codes are appropriate in which situations.

[pramodkvarma]

Fully agree @gamemaker1 about the need to fix error codes and messages to make it useful.

But, when it comes to fully matching HTTP error code to reason, I suggest we not confuse between those two. I am not of the opinion that we should assume APIs are HTTP based at all (many times APIs are called via alternate async route and we must ensure the response structure is not dependent on HTTP headers)! When using HTTP as the protocol of choice to invoke the API, sure, we can use HTTP error as a generic high level response with a more specific error code in reason field.

But, I agree with you on the need to fix the API response structure in the case of any functional error with clearly defined error codes in the reason field (not matching to HTTP error, rather an error code enum value) and a clear message in message field (as you suggested).

Can you help define the reason codes as enums for registry and credentialing micro-services? That'd be great!

[gamemaker1]

Clearly defined error codes in the reason field (not matching to HTTP error, rather an error code enum value) and a clear message in message field (as you suggested).

That sounds good to me 👍🏾

Can you help define the reason codes as enums for registry and credentialing micro-services?

Sure, I would love to do that!

[gamemaker1]

Hi @pramodkvarma,

As you suggested, I have created a general HTTP response format for the registry server below, including operation types, error locations and error reason codes. Please let me know if I have missed any errors that might be returned by the server, or anything that can be improved upon.

Thanks!

---

General Response Format

// == Headers ==

// HTTP/1.1 {code} {status}
// X-Server: {registry-server-url}
// X-Request-Id: {request-id}

// == Response Body ==

{
  // If an error occurs
  "error": {
    // HTTP status code
    "code": {code},
    // HTTP status text
    "status": "{status}",
    // Summary of error messages in the `errors` array
    "message": "{errors-summary}",
    // List errors that occurred, one by one
    "errors": [
      {
        // Operation being carried out. Can be one of the following:
        // - `CREATE_ENTITY`
        // - `RETRIEVE_ENTITY`
        // - `UPDATE_ENTITY`
        // - `DELETE_ENTITY`
        // - `MAKE_CLAIM`
        // - `ATTEST_CLAIM`
        // - `REJECT_CLAIM`
        "operation": "{operation}",
        // Location of error. Used when there is a problem with the request
        // (else it's value should be null). For example, if there is a
        // missing field `email` in request body, the `location` should be
        // `request.body.email`. If the problem is a missing/invalid token in
        // the authorisation header, the `location` should be
        // `request.headers.authorization`. Can be one of the following:
        // - `request.path.{field}` (URL params) => 400
        // - `request.query.{field}` (Query params) => 400
        // - `request.body.{field}` (Request body) => 400
        // - `request.headers.{field}` (Request headers) => 401/403
        // - `null`
        "location": "{location}",
        // Detailed error message that describes what the error is, and how it
        // can be fixed. For example, for an INVALID_AUTH_TOKEN error, we might
        // return the following error message: 'The auth token specified in the
        // `Authorization` header is invalid or has expired. Try refreshing the
        // token and retrying the same request with the new auth token.'
        "message": "{error-message}",
        // Machine-parsable reason. Can be one of the following:
        // - `SCHEMA_VIOLATION` => 400
        // - `IMPROPER_PAYLOAD_TYPE` => 400
        // - `INVALID_CLAIM_SPECIFIED` => 400
        // - `INVALID_AUTH_TOKEN` => 401
        // - `UNAUTHORIZED_FOR_OPERATION` => 403
        // - `ROUTE_NOT_FOUND` => 404
        // - `ENTITY_NOT_FOUND` => 404
        // - `OPERATION_NOT_ALLOWED` => 405
        // - `BACKEND_ERROR` => 500, 502, 503 or 504
        "reason": "{error-reason}"
      }
    ]
  },
  // If the operation is successfull
  "data": {
    // ...
  }
}

Operation

Name	Description
CREATE_ENTITY	Create an entity
RETRIEVE_ENTITY	Retrieve an entity
UPDATE_ENTITY	Update an entity
DELETE_ENTITY	Delete an entity
MAKE_CLAIM	Make a claim and send it for attestation
ATTEST_CLAIM	Attest a claim made by an entity
REJECT_CLAIM	Reject a claim made by an entity

Location

Name	Description	Associated HTTP status code
request.path.{field}	An error occurred while parsing the given field in the URL params	400
request.query.{field}	An error occurred while parsing the given field in query params	400
request.body.{field}	An error occurred while parsing the given field in the request body	400
request.headers.{field}	An error occurred while parsing the given field in the request headers	401/403
null	Not an error related to the request	any

Reason

Name	Description	Associated operation	Associated HTTP status code
SCHEMA_VIOLATION	The payload did not match the entity's schema	CREATE_ENTITY and UPDATE_ENTITY	400
IMPROPER_PAYLOAD_TYPE	The type of the payload did not match the type specified in the schema	MAKE_CLAIM	400
INVALID_CLAIM_SPECIFIED	The _osClaimId is invalid	ATTEST_CLAIM and REJECT_CLAIM	400
INVALID_AUTH_TOKEN	The auth token specified in the authorization header is invalid/expired	RETRIEVE_ENTITY, UPDATE_ENTITY, MAKE_CLAIM, ATTEST_CLAIM, REJECT_CLAIM	401
UNAUTHORIZED_FOR_OPERATION	The user making the request lacks sufficient permissions to carry out the operation	RETRIEVE_ENTITY, UPDATE_ENTITY, MAKE_CLAIM, ATTEST_CLAIM, REJECT_CLAIM	403
ROUTE_NOT_FOUND	404	*	404
ENTITY_NOT_FOUND	The entity being retrieved/updated/etc does not exit	RETRIEVE_ENTITY, UPDATE_ENTITY, MAKE_CLAIM, ATTEST_CLAIM, REJECT_CLAIM	404
OPERATION_NOT_ALLOWED	This operation is not permitted by the server	DELETE_ENTITY	405
BACKEND_ERROR	An internal server error occurred	*	500, 502, 503 and 504

[gamemaker1]

@pramodkvarma any update? Is there anything I can do to help with the implementation of this?

[pramodkvarma]

Go ahead and bring this in as PR @gamemaker1. @dileepbapat can help review and merge it in.

[gamemaker1]

Will do!