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

Tracking issue: better user facing error reporting #683

Closed
2 of 4 tasks
MarinPostma opened this issue May 14, 2020 · 11 comments
Closed
2 of 4 tasks

Tracking issue: better user facing error reporting #683

MarinPostma opened this issue May 14, 2020 · 11 comments
Labels
feature request & feedback Go to https://github.com/meilisearch/product/

Comments

@MarinPostma
Copy link
Contributor

MarinPostma commented May 14, 2020

Introduction

This issue tracks progress being made towards better error reporting.
Currently, when an error occurs, a json object containing a message is sent back to the user, and an HTTP error code is set. The current state of our error reporting is very basic, and our messages tend to lack "user friendliness". With this issue, we'll address this.

Error will be made of 3 parts, described thereafter.

Error codes

Each error that's sent back by the server will be associated with an error code. A nomenclature is TBD, and abstractions on the server-side will enforce this practice. This will allow easier error parsing, by making the error independent from it's message, also resulting in better SDK maintainability.

The error code will be reported thanks to the errorCode attribute in the response document.

discussions

  • include the error code in the header, so parsing of the JSON document is not mandatory anymore.
  • find a good nomenclature for error codes

checklist

  • list all existing error and categorize them
  • determine a nomenclature

Error message

Not much change on this side for now. More effort will be put into this in a later time to provide better error messages (maybe thanks to Anyhow?).

The error message will be reported thanks to the errorMessage attribute in the response document.

Error links

A link to the documentation will also be returned with the error. It will link to the error page (to be created) to the section corresponding to the error code. This will allow the user to get more information on what failed, and be forwarded the relevant documentation.

A minified link will be provided, in order to remain succinct and readable. It will be in the form:
docs.meilisearch.com/error/$error_code. Internally, a router redirects this request to the relevant error documentation.

The error link will be reported thanks to the errorLink attribute in the response document.

checklist

  • new page in documentation, with all error messages
  • links to each section of the error documentation
@MarinPostma MarinPostma changed the title tracking issue: better use facing error reporting tracking issue: better user facing error reporting May 14, 2020
@eskombro
Copy link
Member

eskombro commented May 14, 2020

Error message

Not much change on this side for now. More effort will be put into this in a later time to provide better error messages (maybe thanks to Anyhow?).

Thanks for this issue, it's really important progress IMO.

For now some SDK methods may rely in the error message (e.g. meilisearch/integration-guides#2), so consider that introducing modifications in error messages could imply breaking changes!

@qdequele qdequele changed the title tracking issue: better user facing error reporting Tracking issue: better user facing error reporting May 14, 2020
@MarinPostma
Copy link
Contributor Author

@eskombro, noted! first we'll introduce error code so you can rely on them, then we'll improve the messages ;)

@MarinPostma MarinPostma added the tracking issue Tracks development of a global issue label May 15, 2020
@MarinPostma
Copy link
Contributor Author

After some discussions with @Kerollmops, we've decided upon the following error schema:

EPFCNN
  • E: plain letter "E", to mark an error code, future main introduce W for warning
  • P: scope of the error, 0 for private, 1 for public. Private are error that make no sense reporting to the user, they are internal errors, and there is nothing the user can do about them. they are nonetheless returned, without a message, for assistance purpose.
  • F: 0 or 1, report if the error is fatal.
  • C: error category, number in 0-9, the category of the error. Categories are still to be determined, input is required.
  • NN: The error number, two digits, within C.

examples:
E01302: a private fatal error of category 3 with id 2
E10413: a public error, non-fatal, of category 4, with id 13

this leaves us plenty of naming space for errors, and we can still use hexa if we happen to need to more categories.

For every error, it should be so that CNN remains constant. The other value can be changed with time, if deemed necessary. SDKs can rely on these values CNN for error reporting, and use E and P to conditionnally report the error. This allows to change error behavior with minimal overhead.

The category I have come up with for now are:
- bad user input
- permission error
- invalid state
- OS error

There are certainly more of them, input would be appreciated. @eskombro @qdequele @curquiza

@eskombro
Copy link
Member

eskombro commented May 18, 2020

Looks great!

The category I have come up with for now are:

  • bad user input
  • permission error
  • invalid state
  • OS error

Simple question, creating an index that already exists, or requesting and index which doesn't, for example, are those kind of errors considered as "bad user input"?
Can you clarify a little bit what do you mean by "invalid state"?

P: scope of the error, 0 for private, 1 for public. Private are error that make no sense reporting to the user, they are internal errors, and there is nothing the user can do about them. they are nonetheless returned, without a message, for assistance purpose.

Also, I suppose you mean using a generic error message, because no message at all seems very harsh on the user :)

and we can still use hexa if we happen to need to more categories.

Smart

GJ!

@MarinPostma
Copy link
Contributor Author

MarinPostma commented May 18, 2020

Also, I suppose you mean using a generic error message, because no message at all seems very harsh on the user :)

yeah, something like "internal error: EXXXXX". That being said, the server will probably return the error message anyway, but this gives an indication to the front that the message should be ignored. Such errors may also not be thoroughly publicly documented, and a generic documentation will cover them all at once..

Simple question, creating an index that already exists, or requesting and index which doesn't, for example, are those kind of errors considered as "bad user input"?

this is TBD, and I'd like your input on the matter

Can you clarify a little bit what do you mean by "invalid state"?

Those are state you should not find yourself in, such as nonexistent schema, or things like that. These errors may very well all be private. We can also imagine different level of error reporting depending on who asks, the admin may receive more info than a generic user. The schema allows that.

@curquiza
Copy link
Member

curquiza commented May 18, 2020

I like your suggestion 🙂

For every error, it should be so that CNN remains constant. The other value can be changed with time, if deemed necessary. SDKs can rely on these values CNN for error reporting, and use E and P to conditionnally report the error. This allows to change error behavior with minimal overhead.

It means that, in the future, the error code could be longer, but the P following E will stay, and also the three last characters CNN, am I right? With other words: the SDK should check the 2 first characters and the 3 last ones.

There are certainly more of them, input would be appreciated.

I have no other category in my mind right now... Be sure I'll keep thinking about that.
We might need a category for the 404 error, for example, if we try to call the route DELETE /indexes (which does not exist).
And if I understand well the primary key errors would be as invalid state, right?

Simple question, creating an index that already exists, or requesting and index which doesn't, for example, are those kind of errors considered as "bad user input"?

this is TBD, and I'd like your input on the matter

For me, with the 4 current categories, it would be invalid state, but we definitely should discuss that. (We should not create a category just for this error)

@MarinPostma
Copy link
Contributor Author

It means that, in the future, the error code could be longer, but the P following E will stay, and also the three last characters CNN, am I right? With other words: the SDK should check the 2 first characters and the 3 last ones.

I certainely hope they won't, but this is not what I meant. CNN for an error (the last 3 digits as you pointed out) will never change. However, we may very well decide to change a fatal error to become non fatal, or a private one to become public, so P and F can change over time. That means that you the only thing you can count on not to change is CNN, the rest should be logically checked.

And if I understand well the primary key errors would be as invalid state, right?

exactly

@curquiza
Copy link
Member

curquiza commented May 18, 2020

I certainely hope they won't, but this is not what I meant. CNN for an error (the last 3 digits as you pointed out) will never change. However, we may very well decide to change a fatal error to become non fatal, or a private one to become public, so P and F can change over time. That means that you the only thing you can count on not to change is CNN, the rest should be logically checked.

Okaaay I indeed did not understand well! It's clear now for me! Great then! 🙂

@MarinPostma
Copy link
Contributor Author

After discussions with the team, we decided to change the error code reporting scheme, for a more user friendly one. This is inspired by the way stripe deals with error codes.
The payload in case of error look like this

{
            "message": "...",
            "errorCode": "...",
            "errorType": "...",
            "errorLink": "...",
}

We currently have 3 error types:

  • InternalError: error that are not related with what the used inputed
  • InvalidRequest: error related with bad user input
  • Authentication: error related to authentication

there are many error codes, each ascociated with an error type, and linking to the documentation with errorUrl

@bidoubiwa
Copy link
Contributor

bidoubiwa commented May 29, 2020

Error types

Proposition for the errorType's

  • AuthenticationError
  • InvalidRequestError
  • InternalError

This is for clarity and consistence between the different types.

  • Clarity: The string in itself is explanatory and does not need the context to be understood.
  • Consistency: Every type has an error aspect to it

Important exception

Special case the index_already_exists error, this way SDKs can base there error handling on that.

@gmourier
Copy link
Member

gmourier commented Aug 9, 2021

Closed because it is now implemented.

@gmourier gmourier closed this as completed Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request & feedback Go to https://github.com/meilisearch/product/
Projects
None yet
Development

No branches or pull requests

6 participants