5 ms·

Here is my favorite solution to this problem // CheckClose is a utility function used to check the return from // Close in a defer statement. func CheckC

• Belphemur 2y ago
  I still question why defer doesn't support doing exactly that. After all it's like the go language provide us with a cleanup function that in 99% of the time shouldn't be used unless we manually wrap what it's calling to properly handle error. In the end, what's the point of defer ?
  • randomdata 2y ago
    > I still question why defer doesn't support doing exactly that. When would it ever be useful? You'd soon start to hate life if you actually tried using the above function in anything beyond a toy application. > 99% of the time shouldn't be used 1. 99% of the time it is fine to use without further consideration. Even if there are errors, they don't matter. The example from the parent comment is a perfect case in point. Who cares if Close fails? It doesn't affect you in any way. 2. 0.999% of the time if you have a function that combines an operation that might fail in a manner you need to deal with along with cleanup it will be designed to allow being called more than once, allowing you, the caller, to separate the operation and cleanup phases in your code. 3. 0.001% you might have to be careful about its use if a package has an ill-conceived API. If you can, fix the API. The chances of you encountering this is slim, though, especially if you don't randomly import packages written by a high school student writing code for the first time ever.
    • bryancoxwell 2y ago
      The whole point of this post is that an error returned from file.Close DOES matter
      • foldr 2y ago
        I think from a Go point of view, the lesson to be drawn from that is "don't defer a function call if you need to check its error value", rather than "defer needs to support checking of function return values". In the example at hand, it really makes more sense to call Close() as soon as possible after the file is written. It's more of an issue with the underlying OS file API making error checking difficult. In 99% of cases, the solution to this problem will be to use a WriteFile function that opens, writes and closes the file and does all the error handling for you.
        • deleted 2y ago
          [deleted]
        • randomdata 2y ago
          > the lesson to be drawn from that is "don't defer a function call if you need to check its error value" Isn't the lesson here: If you must have a Close method that might fail in your API, ensure it can safely be called multiple times? As long as that is true, you can approach it like you would any other API that has resources that might need to be cleaned up. f, _ := os.Create(...) defer f.Close() // perform writes and whatever else if err := f.Close(); err != nil { // recover from failure } (os.File supports this, expectedly) > the solution to this problem will be to use a WriteFile function If it were the solution you'd already be using os.WriteFile. It has a time and place, but often it is not suitable. Notably because it requires the entire file contents to be first stored in memory, which can become problematic. Certainly you could write a custom WriteFile function that is tuned to your specific requirements, but now you're back to needing to be familiar with the intricacies of a lower-level API in order to facilitate that.
          • foldr 2y ago
• saurik 2y ago
  Which of course isn't really a "solution", as it results in ambiguous error semantics; this is especially the case with Go's defer, as it pushes the code to the end of the entire function, not merely some intermediate relevant scope... the result is that a file used near the beginning of a function might fail to close but that error will not be realized until after something else in the function fails, after the point of no return on the close. Really, this is kind of a fundamental limitation of automatic allocation semantics, which is effectively a monad that is being stacked with the error propagation monad in a confusing manner that means you "should" only defer operations which don't have errors. Even in languages with exceptions, such as C++ and Java, they had to wrangle with this problem and failed to solve it: in C++ 11 or 17 or whatever, deconstructors are now by default nothrow in an attempt to prevent this kind of mistake. ...but then, what does one do with close?! In some sense, the entire concept of close must not fail, and yet it exists in a world where we don't really believe in anything that can't fail, as we like moving around failure semantics. FWIW, Linus has suggested that the kernel should largely accept that application developers don't ever check the return value of close... but has also stated that developers should sync the file first if they care and also check close (at least, to be maximally correct). But like, what does one do then? How do you ever recover from this? I actually think you can't, if you are in a cleanup operation... not without breaking your error regime. I think--and this is also where the article eventually goes in the updates (though without noting you can add your own boolean)--you should therefore both explicitly close (and/or maybe flush/sync) the file after writing to it and have a "if I didn't close this, close it" in your cleanup handler; critically, the (explicit) former checks for errors, while the (implicit) latter doesn't.
  • d0mine 2y ago
    The solution is that there are multiple solutions that are suitable in different situations. If you need "strict" requirements then it is likely impossible but you can get close enough if you use sqlite. Or more lightweight atomic/thread-safe/fault-tolerant file library. You could rollout your own: it easy to start, and continue until the error rate is tolerable for your application (though it may take more dev time). If you don't need db-like strict guarantees. Just write your app knowing that it may fail (data may be lost, corrupted). It may be ok in a lot of cases.
• randomdata 2y ago
  >>> f, err := os.Open(blah) // ... defer CheckClose(f, &err) What knowledge do you hope to gain of f.Close fails here?
  • red_admiral 2y ago
    If I understand the OP correctly, if Close() fails then you can't trust that the data was written, even if the previous Write() succeeded.
    • randomdata 2y ago
      That's not it. You can't write to a file handle returned by os.Open. https://pkg.go.dev/os#Open https://pkg.go.dev/os#Open func Open(name string) (*File, error) Open opens the named file for reading. If successful, methods on the returned file can be used for reading; the associated file descriptor has mode O_RDONLY. If there is an error, it will be of type *PathError.
    • Cthulhu_ 2y ago
      Exactly that; for critical operations like e.g. a database, if a write fails you've got corrupted data and you have a Major Issue. That said, I'm not sure how they would handle a file close failure, wouldn't the file be corrupted anyway because some of the bits may have been written? Then again, at least you can raise the alarms if Close fails, because silent failures are worse than failures.
      • randomdata 2y ago
        > if a write fails. We're talking about a read-only case. os.Open returns a read-only file handle. If you try writing to it, you'll get an error already at that point. If close fails, who cares? > I'm not sure how they would handle a file close failure Ideally there is some kind of failover you can resort to, but if there is no other option at very least you will want to notify a human that what they thought was written isn't actually. But when only reading, you don't need to fall back to anything – all the reads were successful – and what is it to the human? What they thought was supposed to happen did!
• sa46 2y ago
  I use a similar pattern, cribbed from one of the Go databases that aspired never to ignore errors. The difference is that errs.Capture preserves existing errors and formats the error with a message--important if generalized to handle any error function. package errs // Capture runs errFunc and assigns the error, if any, to *errPtr. Preserves the // original error by wrapping with errors.Join if the errFunc err is non-nil. func Capture(errPtr *error, errFunc func() error, msg string) { err := errFunc() if err == nil { return } *errPtr = errors.Join(*errPtr, fmt.Errorf("%s: %w", msg, err)) } I conventionally use mErr to distinguish from err. func doThing() (_ string, mErr error) { f, err := os.Open("foo") if err != nil { return "", fmt.Errorf("open file: %w", err) } // Use the file... defer errs.Capture(&mErr, f.Close, "close file") return "", nil }
• joeshaw 2y ago
  I caution against this approach, as you are not really dealing with the error when it occurs. If the work you do after the defer has other side effects, you may have just gotten your application into an inconsistent state and it's very hard to see in code why this might be. `defer` is really not well-suited for error handling, its benefit is mainly in resource cleanup where failure is impossible or doesn't matter. (This makes it fine for `Close` on read-only file I/O operations, and not so great for writes.)
  • stbenjam 2y ago
    > If the work you do after the defer has other side effects Defer is by definition the last work you do in a function, there won't be more work except by the caller who will get the error returned to them. If you are structuring a function that writes a file, and then does something with it, defer isn't appropriate, since you should close it before you do any more work.
    • Cthulhu_ 2y ago
      It's possible to have multiple defers in a function though (so you have multiple "last work in a function"; nowhere is it dictated that a function should only have one operation that needs to clean something up at the end. Think for example copying one file to another.
  • Thorrez 2y ago
    > If the work you do after the defer has other side effects, you may have just gotten your application into an inconsistent state and it's very hard to see in code why this might be. Can you give an example case of how this could happen?
    • joeshaw 2y ago
      This is a contrived example, but imagine a situation where I have a file I want to write on disk and then have a reference to it in a database. If I have a flow like: func UpdateUser(user *User) (err error) { f, err := os.Create("/some/file.txt") if err != nil { return err } defer CheckClose(f, &err) if _, err := f.Write(somedata); err != nil { return err } if err := db.UpdateUser(user, "/some/file.txt"); err != nil { return err } return } This function might have updated the user in the database with a new file despite the fact that `CheckClose` (defined up-thread) does check to see if the `Close` failed and returned an error. The calling code won't have known this has happened. The core problem is that the error checking is not done soon enough, either because Go programmers are conditioned to `defer f.Close()` from nearly all example code -- most of it demonstrating reads, not writes -- or because they are handling the error, but only in a deferred function, not earlier. A more correct way to do this would be: func UpdateUser(user *User) error { f, err := os.Create("/some/file.txt") if err != nil { return err } defer f.Close() if _, err := f.Write(somedata); err != nil { return err } if err := f.Sync(); err != nil { return err } if err := f.Close(); err != nil { return err } if err := db.UpdateUser(user, "/some/file.txt"); err != nil { return err } } `Sync()` flushes the data to disk, and `Close()` gives a "last-chance" opportunity to return an error. The `defer f.Close()` exists as a way to ensure resource cleanup if an error occurs before the explicit `f.Close()` toward the end of the function. As I mentioned in an update to the post, double `Close()` is fine.
• red_admiral 2y ago
  I wouldn't mind the language adding some syntax for this, based on the java try-with-resource but using error values rather than exceptions. Something like f, err = with(os.Open(thing)) { // do stuff } // at this point err has the same semantics as your CheckClose
  • Cthulhu_ 2y ago
    I believe your example doesn't introduce any new syntax though. Go is highly resistant to adding syntax (and that's a good thing, it keeps the tooling fast and barrier to entry low).