Include error code and name on error message #98
Conversation
Ideally we'd expose the error info in a structured way like suggested in trinodb#97 . Since that's more involved as requires changing exported interfaces, in the meantime having the info in the error message might be already helpful for telemetry.
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
| @@ -725,7 +725,7 @@ type stmtErrorFailureInfo struct { | |||
| } | |||
|
|
|||
| func (e stmtError) Error() string { | |||
| return e.FailureInfo.Type + ": " + e.Message | |||
| return fmt.Sprintf("%v (name: %s, code: %d): %s", e.FailureInfo.Type, e.ErrorName, e.ErrorCode, e.Message) | |||
There was a problem hiding this comment.
Can you add a simple test for that? Maybe a query with a syntax error? I'd like to see some example value and make sure it's human-readable and informative.
We should not encourage parsing this string and instead fix the interfaces as suggested in #97.
There was a problem hiding this comment.
Can do, but wouldn't a test encourage folk to rely on the message as if it was some sort of contract 🤔 ?
There was a problem hiding this comment.
I don't think so. I asked for it to have increased coverage and a good example of what's expected there.
There was a problem hiding this comment.
PTAL at #109, maybe it solves getting error details without having to parse the error string.
Ideally we'd expose the error info in a structured way like suggested in #97 . Since that's more involved as requires changing exported interfaces, in the meantime having the info in the error message might be already helpful for telemetry.