Make error message consistent across all backends

[xiaofei-du]

trafficstars

Why I suggest we make the error message consistent across all backends, so the frontend can present more constructive error messages to the users.

Now Right now, our API error returns all error messages in the message field.

{
    "code": 5,
    "message": "a long and detailed error description",
    "details": []
}

How to improve We put a short error description in the message field (generally less than 5 words), and put the long description in details field. Use the above error as an example.

{
    "code": 5,
    "message": "short error description",
    "details": ["a long and detailed error description"]
}

Btw, From Golang docs,

Error strings should not be capitalized (unless beginning with proper nouns or acronyms) or end with punctuation, since they are usually printed following other context. That is, use fmt.Errorf("something bad") not fmt.Errorf("Something bad"), so that log.Printf("Reading %s: %v", filename, err) formats without a spurious capital letter mid-message. This does not apply to logging, which is implicitly line-oriented and not combined inside other messages.

Let's follow the practice in refactoring.

• [ ] Pipeline-backend @pinglin
• [ ] Connector-backend @pinglin
• [ ] Model-backend @Phelan164
• [x] Mgmt-backend @xiaofei-du (https://github.com/instill-ai/mgmt-backend/pull/11)

[pinglin]

API operation behaviour across backends includes

1. DELETE a resource

• Return 422 if it is occupied by other resources
• Returns 5xx (usually 500) for server error
• Otherwise, return 200

2. CHANGE the state of a resource (e.g., (un)deploy a model, (dis)connect a connector)

• Returns 422 when the resource state is STATE_ERROR
• Returns 422 when the resource does not allow this operation (e.g., the connector is a directness connector, that only allows a STATE_CONNECTED state
• Returns 5xx (usually 500) for server error
• Otherwise, returns 200
  • this 200 only means that the server accepts your request, it does not mean the requested operation is successful.
  • if the operation intends to change the resource state to X, but the state of the resource is already X, still return 200

3. CREATE a resource

• Returns 400 when the request body does not pass validation from the backend, e.g.,
  • wrong id format
  • missing a required field
• Returns 409 when the resource is duplicated
• Returns 5xx (usually 500) for server error
• Otherwise, returns 200

4. GET a resource

• Returns 404 when the resource is not found
• Returns 5xx (usually 500) for server error
• Otherwise, returns 200

5 UPDATE a resource

• Returns 404 when the resource is not found
• Returns 400 when the request body does not pass validation from the backend, e.g.,
  • wrong id format
  • updating an immutable field with a different value
• Returns 5xx (usually 500) for server error
• Otherwise, returns 200

6 LIST a group of resources

• Returns 5xx (usually 500) for server error
• Otherwise, returns 200

[pinglin]

Notes for backend owners to refactor the response error:

• We adopt the Error model of Google API. A gRPC-Gateway REST error response thus has content-type: application/problem+json for the code, message, and details field.
• To overwrite the gRPC-Gateway default error handler (hint from this thread), we need to implement the custom logic. Please refer to instill-ai/connector-backend@2fc9487.
• gRPC-Gateway has the default HTTP and gRPC mapping. If you want to change the default mapping, implement the logic in the errorHandler like this.
• Detailed status errors are shared in instill-ai/x/sterr and use it as
  • refer to instill-ai/connector-backend@94f40ab
  • code field is for gRPC code
  • message field can be used to indicate function location, i.e., [db], [service] or [handler], or [<external-service-name>] when it's a external service error response
  • details field is used according to the detailed error types. We currently use only BadRequest, PreconditionFailure and ResourceInfo type.
  • BadRequest is mostly for HTTP 400 Bad Request and the corresponding gRPC InvalidArgument
  • PreconditionFailure is for HTTP 422 preconditioning on user errors
  • ResourceInfoStatus is for internal and external resource access errors
  • For error response without details array filled, use google.golang.org/grpc/status's status.Error directly.

[xiaofei-du]

Regarding refactoring the errors in backends: all our backends have 3 layers:

• handler
• service
• repository

To make the refactor simpler, I suggest we

• only use the "github.com/instill-ai/x/sterr" in the handler layer.
• service and repository layers will return normal errors like status.Errorf(…) from "google.golang.org/grpc/status” to the handler layer.

So the handler layer will be the central place to decide how to return the final user-facing error with "github.com/instill-ai/x/sterr", and we don’t have to refactor the error handling in all three layers all over the place.

[felixcorleone]

@xiaofei-du @pinglin Considering that the latest update time for this issue was over five weeks ago, we are closing it. If there are any updates in the future, please reopen the issue and provide the latest requirements and status. Thank you.