search

cockroachdb / errors

Go error library with error portability over the network
Apache License 2.0
2.09k stars 67 forks source link

support wrapError encode and decode #113

Closed dhartunian closed 1 year ago

dhartunian commented 1 year ago

Previously, support for the wrapError type generated by fmt.Errorf("%w xxxx") was missing an understanding of the fact that the format string wasn't limited to just prefixes. Hence the opaqueWrapper struct assumed that all wrappers had prefix strings that would precede their causal chains.

This change adds a flag to the EncodedWrapper which tracks whether the wrapper "owns" the error string. In the case of fmt.Errorf the wrapError struct created within the stdlib contains a fully interpolated string as its message, not the format string, or a prefix. This means that when we encode it into a EncodedWrapper for transfer over the network, we need to remember to just print the wrapper's string when .Error() is called on the decoded object on the other end.

Paired with this change the opaqueWrapper that is generated on the decode side now contains this field as well which is referenced when calling .Error() on the wrapper, ensuring we don't re-concatenate the causes. This field is also referenced when formatting the error in state.formatSingleLineOutput where we use it to simply output the final entry's message, instead of concatenating all the entries together.

This change is backwards compatible since older versions will omit the new field in the proto, defaulting it to false in newer versions, which will preserve existing behavior when printing out errors. New code will set the field when necessary and interpret it appropriately. If an error encoded with a new version is decoded by an older version, the information about the error string will be discarded, which may lead to repeated portions of the error message when printed out, if the wrapper contains a copy of the message of its cause.

This change is Reviewable

knz commented 1 year ago

Also maybe mention the new api in the README.

knz commented 1 year ago

I am definitely missing a lot of details, but why can't we remember the format string (with %ws in it) and fill in the %ws when we format the wrapper?

By the time our errors library looks at an error object, that format string is long gone (it should work with error types not defined through it).