-
Notifications
You must be signed in to change notification settings - Fork 38
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
Fix/Unwrap incomplete object put errors #2126
Conversation
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3763 |
Codecov Report
@@ Coverage Diff @@
## master #2126 +/- ##
==========================================
- Coverage 30.78% 30.53% -0.26%
==========================================
Files 382 381 -1
Lines 28056 28003 -53
==========================================
- Hits 8637 8550 -87
- Misses 18687 18721 +34
Partials 732 732
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
I am running integration tests |
Test run is finished. Please download the tarball from link. Untar and use |
@@ -115,6 +115,10 @@ func (x errIncompletePut) Error() string { | |||
return commonMsg | |||
} | |||
|
|||
func (x errIncompletePut) Unwrap() 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.
Can you give an example of what message is expected now? Is it still obvious that there was some error on others node.
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.
With that PR error messages would be just the deepest error because the current error messaging is designed to be so
I agree that keeping the chain could be a good error context and that is why im not sure about the PR. The problem that PR solves is described in the linked issue: any SDK error that has not been converted to its V2 form does not produce a text message (wrapping an SDK error by an error that does not have Unwrap
method leads to calling Error
method without conversion to a V2
struct => no error message). I did not like such a solution but here we are.
If our desire to save error chain in an "incomplete" context is really strong, I may suggest using init()
in SDK for predefined errors that fills all error messages before their usage. Another solution could be introducing a new "IncompleteObjectPut" API error that wraps any error and has a detailed description of error numbers and reasons\
/cc @cthulhu-rider
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.
For now I don't mind
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3770 |
I am running integration tests |
Test run is finished. Please download the tarball from link. Untar and use |
Closing this in favor of nspcc-dev/neofs-sdk-go#369. |
Closes #2092.
Before:
After:
Not sure if we want to lose error chain but that is how unwrapping (looking for API status errors) works in the current code base.