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

implement Exception.Unwrap method #342

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Conversation

husio
Copy link
Contributor

@husio husio commented Oct 10, 2023

In order to make exceptions testable with errors.Is, they must implement the Unwrap method to allow access to the underlying error code(s).

This change is addressing the problem of the below code not working as expected, when testing returned by Do method error:

c, _ := ch.Dial(ctx, ch.Options{...})
switch err := c.Do(ctx, ch.Query{...}); {
case err == nil:
	// ...
case errors.Is(err, proto.ErrTableIsReadOnly), errors.Is(err, proto.ErrReadonly):
	// ... <- Never executed, because exception unwraping is not implemented.
default:
    // ...
}

Because Exception instances are created dynamically, unwrapping can ignore nested exception instances and collect only the underlying error codes.

@CLAassistant
Copy link

CLAassistant commented Oct 10, 2023

CLA assistant check
All committers have signed the CLA.

@serprex serprex force-pushed the exception_unwrap branch 2 times, most recently from cbb717d to 57d2bd4 Compare October 14, 2024 06:50
In order to make exceptions testable with errors.Is, they must implement
the `Unwrap` method to allow access to the underlying error code(s).

This change is addressing the problem of the below code not working as
expected, when testing returned by `Do` method error:

    c, _ := ch.Dial(ctx, ch.Options{...})
    switch err := c.Do(ctx, ch.Query{...}); {
    case err == nil:
    	// ...
    case errors.Is(err, proto.ErrTableIsReadOnly), errors.Is(err, proto.ErrReadonly):
    	// ... <- Never executed, because exception unwraping is not implemented.
    default:
        // ...
    }

Because `Exception` instances are created dynamically, unwrapping can
ignore nested exception instances and collect only underlying error codes.
@serprex serprex merged commit e52bd23 into ClickHouse:main Oct 14, 2024
25 checks passed
@husio husio deleted the exception_unwrap branch October 19, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants