authgear / authgear-server

Open source alternative to Auth0 / Firebase Auth
https://www.authgear.com
Apache License 2.0
81 stars 37 forks source link

Improve Error Response in APIs #4422

Open louischan-oursky opened 3 months ago

louischan-oursky commented 3 months ago

We shall improve the error response at once, currently, there are several problems:

  1. name is type of error which is confusing.
  2. code is just HTTP status code, which doesn't have to be in JSON response.
  3. info doesn't have standardized fields or documentation.

So instead of:

"error": {
    "name": "Invalid",
    "reason": "InvariantViolated",
    "message": "identity already exists",
    "code": 400,
    "info": {
      "cause": {
        "kind": "DuplicatedIdentity"
      }
    }
}

We shall change to:

"error": {
  "type": "TypeOfError"
  "code": "Short String for the Error"
  "message": "Human readable message"
  "info": { }
}

The main changes from the current design are:

Original rename suggested by Louis

linear[bot] commented 3 months ago

DEV-1501 Rename some important errors

chpapa commented 3 months ago

I think we shall improve the error response at once, currently there are several problems:

  1. name is type of error which is confusing.
  2. code is just HTTP status code, which doesn't have to be in JSON response.
  3. info doesn't have a standardized fields or documentations.

So...

  1. Besides this API and Authflow, where is it being used?
  2. I think instead of:
"error": {
    "name": "Invalid",
    "reason": "InvariantViolated",
    "message": "identity already exists",
    "code": 400,
    "info": {
      "cause": {
        "kind": "DuplicatedIdentity"
      }
    }
}

We shall change to:

"error": {
  "type": "TypeOfError"
  "code": "Short String for the Error"
  "message": "Human readable message"
  "info": { }
}

The main changes from current design are:

chpapa commented 3 months ago

@louischan-oursky I think part of the requirements of a good error handling is actually enable @buildbro able to write an "Error" page under https://docs.authgear.com/reference/apis

louischan-oursky commented 3 months ago

Use type instead of name for category of error. Name is very confusing. We also should have a list of type possible values.

The SDK is an existing consumer of the error object. We cannot just rename it. The best we can do is to repeat name as type, but we probably cannot remove name.

HTTP status code should go back to HTTP response header

The HTTP response header has the same status code. Again we cannot remove the field.

code and message are the actual error

Your proposed code is the original reason. message are the same, human-readable message. code is already taken, and we cannot remove it.

We shall study the APIs used this error scheme, and try to come up with a standardized object for info, otherwise it is not usable at all (or else we need to write a spec to indicate what error will have which info, but it is not as good as a standardized object.

The info is supposed to contain error specific information. If the error object has no specific information, then it has no info.