-
Notifications
You must be signed in to change notification settings - Fork 33
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
fixed #82 Return error when CopyTo/MoveTo functions are called when S… #83
Conversation
…eek offset is not (0,0) for all backends, not just GCS
@@ -29,7 +30,7 @@ type File struct { | |||
func (f *File) Close() error { | |||
if f.tempFile != nil { | |||
defer func() { | |||
f.tempFile.Close() | |||
_ = f.tempFile.Close() |
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.
we would never want to do anything with these errors?
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.
good question... here i'm not changing any behavior except making it explicit what we're doing. Close() is really the hardest thing to do right. Either you run Close() in a defer or you rerun it at every exit point of the function. But each exit point of the function either returns an error already or nil (no error). Unfortunately, we can't return a value (or error) from inside defer, without potentially overwriting an error outside of the defer. Furthermore, we can't really return 2 errors without concatenating them somehow.
The particular quandary of handling errors on Close() doesn't really seem to have a community standard or idiomatic approach. We made a stab at it early using a error type that allowed us to append errors to AND utilized named error returns so that both the normal code and Close() defer closure would update the error. You can see the code at https://github.com/C2FO/vfs/blob/8558f12c8f4cb571cd32a610936f0c1d48aa737b/errors.go and example usage at https://github.com/C2FO/vfs/blob/8558f12c8f4cb571cd32a610936f0c1d48aa737b/errors_example_test.go. However we soon found this a cumbersome and kludgy solution with very little gain. Close() is the one return value that we regularly ignore.
I'm all for a better solution though!
fixes #82
Return error when CopyTo/MoveTo functions are called when Seek offset is not (0,0) for all backends, not just GCS