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

Log context/traceback for errors #912

Open
smacker opened this issue Jul 2, 2019 · 3 comments
Open

Log context/traceback for errors #912

smacker opened this issue Jul 2, 2019 · 3 comments
Labels
blocked Some other issue is blocking this question Further information is requested

Comments

@smacker
Copy link

smacker commented Jul 2, 2019

I keep getting unknown error: object not found but there are no logs about this error for debugging.

Example:

$ docker exec -it 8633920ca611 mysql
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MySQL connection id is 50
Server version: 5.5.10-Vitess

Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MySQL [(none)]> SELECT commit_author_name FROM commits;
ERROR 1105 (HY000): unknown error: object not found

But all I see in the logs:

time="2019-07-02T13:47:04Z" level=debug msg="executing query" query="SELECT commit_author_name FROM commits"
time="2019-07-02T13:47:04Z" level=info msg="audit trail" action=authorization address="127.0.0.1:60092" connection_id=50 permission=read pid=422 query="SELECT commit_author_name FROM commits" success=true system=audit user=root
time="2019-07-02T13:47:05Z" level=info msg="audit trail" action=query address="127.0.0.1:60092" connection_id=50 duration=1.7181277s err="object not found" pid=422 query="SELECT commit_author_name FROM commits" success=false system=audit user=root
@ajnavarro ajnavarro added the question Further information is requested label Jul 3, 2019
@erizocosmico
Copy link
Contributor

erizocosmico commented Jul 3, 2019

Errors are not logged. They are returned and sent to the client. Recently we merged a PR adding a on repository "foo" to all errors so it can be traced back to a specific repository.

I agree, though, that errors, specially git errors, should come with a lot more information (aka, don't just passing errors down without adding info).
Sadly, go-git does not give much info in the errors (probably because of the use of sentinel errors), so we'd have to provide it ourselves.

@smacker
Copy link
Author

smacker commented Jul 3, 2019

jfyi in this case on repository "foo" wouldn't help because I have only 1 repository in my dataset. But with current errors, I still have no idea what is going on.

@erizocosmico erizocosmico self-assigned this Aug 2, 2019
@erizocosmico
Copy link
Contributor

erizocosmico commented Aug 2, 2019

I've been thinking about how to approach this and have more meaningful errors that contain all the possible info.

What I think we should do is to not just return nil, err anymore, but (except where it does not make sense) always annotate it. Like, take for example any expression Eval method. Instead of just returning, some fmt.Errorf("couldn't get expression: %s", err) would be better and it's way more self-explanatory.

The problem here:

  • Using fmt.Errorf loses track of the previous error and just converts it to text. Particularly with go-errors errors we do not want this, because we would also lose the stack trace.
  • Using go-errors means we need to create a new kind for every single error message we want to wrap some error with. To do errCantGetExpression.Wrap(originalErr) we need to also do var errCantGetExpression = errors.NewKind("can't get expression"). This is going to potentially require creating hundreds of new errors (and I don't think it's an exaggeration) simply for annotating. They're not errors we might want to check later for its type, they're just additional info added to the error. So this seems like an overkill. Also, if we use go-errors and the root error is not an error created with this library, there is no (easy) way to check for it.
  • xerrors has xerrors.Errorf which wraps an error without needing a new type or kind and you can later check with xerrors.Is, but it's not to be released until Go 1.13. For now it's an experimental API and it might change. Aside from that, xerrors requires you to create your own types, which is more cumbersome than creating kinds.

Conclusion: all three approaches have advantages and shortcomings and there is not a single one that satisfies our needs to make this not a total PITA.

Since we're in control of go-errors, perhaps we can address these shortcomings by implementing a couple features in it:

Bonus solution: make an internal package in go-mysql-server that gitbase can also use implementing the proposed functions and aliasing the types and functions in go-errors so that the library can be viable for our usecase.

WDYT @src-d/data-processing @smola?

@erizocosmico erizocosmico added the blocked Some other issue is blocking this label Aug 5, 2019
@erizocosmico erizocosmico removed their assignment Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Some other issue is blocking this question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants