-
Notifications
You must be signed in to change notification settings - Fork 60
fix: returning comparable error on contex's cancelled errorcase #475
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
Conversation
* Module version update from v2 to v3, * Go version dep updated from 1.20 to 1.24, * Added stub record for migration guide.
Add support for `go-option` to the `arrow`, `datetime`, `decimal`, and `uuid` packages. This allows for handling optional values of the `Arrow`, `Datetime`, `Interval`, `Decimal`, `UUID` and `BoxError` types. The following changes have been made: - Added `go:generate` directives to the `Arrow`, `Datetime`, `Interval`, `Decimal`, `UUID` and `BoxError` types to generate optional types using `github.com/tarantool/go-option/cmd/gentypes`. - Implemented `MarshalMsgpack` and `UnmarshalMsgpack` methods for the `Arrow`, `Datetime`, `Interval`, and `Decimal` types. - Added `marshalUUID` and `unmarshalUUID` functions for the `UUID` type. - The generated `_gen.go` files contain the `Optional*` types that wrap the original types and provide methods to handle optional values, including `EncodeMsgpack` and `DecodeMsgpack` for `msgpack` serialization. - Refactored the `datetime` and `decimal` decoders to use the new `UnmarshalMsgpack` methods.
35999b5 to
99a30a9
Compare
Revised error use, added new type CodeErrors to compare them using errors.Is(). Also used to check that request's ctx was cancelled. Fixes (#457)
99a30a9 to
6b1bbe6
Compare
|
Please, change the base branch to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, add support of ability to unwrap/compare an original context error instead of the code.
We should have an entry in the CHANGELOG.md for the change too.
| // Output: | ||
| // Ping Resp data [] | ||
| // Ping Error context is done (request ID N) | ||
| // Ping Error context is done (request ID N) (NxN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format above should replace only the request ID, please fix the replace pattern.
| // Ping Error context is done (request ID N) (NxN) | |
| // Ping Error context is done (request ID N) (0x4008) |
| if !contextDoneErrRegexp.Match([]byte(err.Error())) { | ||
| t.Fatalf("Failed to catch an error from done context") | ||
| } | ||
| // checking that we could use errors.Is to get known about error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // checking that we could use errors.Is to get known about error. | |
| // Checking that we could use errors.Is to get known about error. |
| t.Fatalf("Failed to catch an error from done context") | ||
| } | ||
| // checking that we could use errors.Is to get known about error. | ||
| require.True(t, errors.Is(err, ErrCancelledCtx)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem of the approach - we unable to get an original context error here. But we should.
| require.True(t, errors.Is(err, ErrCancelledCtx)) | |
| require.True(t, errors.Is(err, ErrCancelledCtx)) |
Revised error using, added new type CodeErrors to compare them using errors.Is(). Also used to check that request's ctx was cancelled. Also updated state CodeErrors.
Fixes (#457)
I didn't forget about (remove if it is not applicable):
Related issues: (#457)