Log context/traceback for errors
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
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.
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.
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.Errorfloses 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-errorsmeans we need to create a new kind for every single error message we want to wrap some error with. To doerrCantGetExpression.Wrap(originalErr)we need to also dovar 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.Errorfwhich wraps an error without needing a new type or kind and you can later check withxerrors.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,xerrorsrequires 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:
- A way to check for errors created using something that's not go-errors
- A way to create a new go-errors error just giving text and not needing to create a new kind when it's just for annotating. Something like:
return nil, errors.Wrap(err, "adding more: %s", "info")
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?