Don't defer Close() on writable files (2017)
joeshaw.org
joeshaw.org
- if you are creating a file, to ensure full synchronisation you also need to fsync the parent directory, otherwise the file can be fsynced but the update to the directory lost
- if sync fails, you can not assume anything about the file, whether on-disk or in memory, critically one understanding which got dubbed "fsyncgate" and lead to many RDBMS having to be updated is that you can not portably retry fsync after failure: the earlier error may have invalidated the IO buffers but IO errors may not be sticky, so a later fsync will have nothing to write and report success
That is my understanding.
> Also, how many levels of parents do you need to fsync?
Only one, at least if you didn't create the parent directory (if you did then you might have to fsync its parent, recursively). The fsync on the parent directory ensures the dir entry for your new file is flushed to disk.
Is this some Linux/Unix specific thing? Am I blind?
For cross-platform stuff I've mainly used Boost, which I assumed handled such details.
Also these things are needed very very rarely (which is why few even know about the issue) and are not good for performance and battery life.
Windows also doesn't guarantee that data is written to disk by the time WriteFile/CloseHandle returns, the Windows version of fsync is FlushFileBuffers.
* technically it's the filesystem as much as the OS that is relevant here.
If your processing files from other systems on NTFS you'll very likely have rename said files in an application and store an index of the names.
Windows/NTFS is a different world, there are still edge cases that can go wrong but I don't think this particular one is a problem because FAT/NTFS is not inode-based.
I imagine if you looked at the SQLite source code you'd see different edge-case-handling code for different OSes.
The thing about Windows is that because the file open operation (`CreateFile*()`) by default prevents renames of the file, Windows apps have come to not depend so much on file renaming, which makes one of the biggest filesystem power failure hazards less of an issue on Windows. But not being able to rename files over others as easily as in POSIX really sucks. And this doesn't completely absolve Windows app devs of having to think about power failure recovery! "POSIX semantics" is often short-hand for shortcomings of POSIX that happen to also be there in the WIN32 APIs, such as the lack of a filesystem write barrier, file stat-like system calls that mix different kinds of metadata (which sucks for distributed filesystems protocols), and so on. And yes, you can open files in Windows such that rename-over is allowed, so you still have this problem.
Consider S3-like protocols: these recognise that 99% of the time applications just want “create file with given contents” or “read back what they’ve previously written.”
The edge cases should be off the beaten path, not in your way tripping you up to when you want the simple scenario.
What features do you have in mind?
> It puts the choice of guaranteed writes vs performance to the developer.
Yes, and it's a completely false choice. This entire point of this thread is that fsync is an incredibly difficult API to use in a way that gets you the guarantees you need ("don't lose the writes to this file"). And that the consistency guarantees of specific filesystems, VFS, POSIX, and their interactions are not easy to understand even for the experienced -- and it can be catastrophic to get wrong.
It isn't actually a choice between "Speed vs correctness". That's a nice fairy tale where people get to pretend they know what they're up against and everyone has full information. Most programmers aren't going to have the attention to get this right, even good ones. So then it's just "99.9% chance you fucked it up and it's wrong" and then your users are recovering data from backups.
For example, write groups or barriers (like memory barriers) would be wonderful. Or a transaction api, or io completion ports like on windows.
In a database (and any other software designed for resiliency), you want the file contents to transition cleanly from state A to B to C, with no chance to end up in some intermediate state in the case of power loss. And you want to be notified when the data has durably written. It’s unnecessarily difficult to write code that does that on top of POSIX in an efficient way. Most code that interacts with files is either slow, wrong or both. All because the api is bad.
It'd be very nice to get some sort of async filesystem write barrier API. Something like `int fbarrier(int fd)` such that all writes anywhere in the filesystem will be sync'ed later when you `fsync()` that fd.
It would also be very nice to have an async `sync()`/`fsync()`. That may sound oxymoronic, but it's not. An async `sync()`/`fsync()` would schedule and even start the sync and then provide a completion notice so the application can do other work while the sync happens in the background. One can do sync operations in worker threads and then report completion, but it'd be nice to have this be a first class operation. Really, every system call that does "I/O" or is or can be slow should be / have been designed to be async-capable.
The confusion stems from people thinking that files and directories are more different than they are. Both are inodes, and both are basically containers for data. File inodes are containers for actual data, while directory inodes are containers for other inodes.
All inodes need to be fsynced when you write to them. For files this is obviously when you write data to them. For directories, this is any time you change the inodes they contain, since you’re effectively writing data to them.
You only need to sync the direct parent because the containers aren’t transitive; the grandparent directory only stores a reference to the parent directory, not to the files within the parent directory. It’s basically a graph.
do you mean it's a basically a tree? Because if it were just a graph, you could still have edges from the grandparent to the grandchild in addition to the one from the former to the child
It used to be possible ages ago to hard link to directories, which meant that you could have actual cycles and a recursive tree-walking algorithm would never terminated. (As far as I know you can still do this by editing the disk manually, although I think fsck will make a fuss if it detects this.)
You can still, with the right syscalls and drivers, do something like hard links on NTFS (I think they're technically called mount points but it's not the same thing as POSIX ones). I'm not sure if you can still make directory cycles, and you're probably a bad person if you do.
More than DAGs, but instead pretty arbitrary graphs since you can express cycles with hard links.
You can't if you only allow hard links to files.
It does not.
At best it will schedule a journal commit asynchronously (I recall that ext4 maintainer complained about adding this "workaround for buggy user code" on lkml). If you want to receive an IO error when renaming fails, make sure to call fsync() yourself.
Uh? No.
You can imagine them as a list of name,inode tuples.
The inodes themselves are more like free floating anonymous objects independent of any particular directory, and might not have a named reference at all (O_TMPFILE; or the named reference was deleted but a file descriptor is still open; or orphaned inodes due to filesystem corruption or someone forgetting to fsync the directory after creating a file as masklinn pointed out - e2fsck will link these in /lost+found). This is also why chmod appears to affect other hard links in different directories: Because it actually modifies the inode (file permissions are recorded in that), not the named reference.
And I think it makes the collection/container terminology distinction sharper too. Depending on context, I think it’s usually reasonable (if imprecise) to describe a bucket of references or pointers to things as a collection of those things. But I don’t think it makes as much sense to call it a container of those things, except in a really abstract sense.
There's a reason Apple made fsync useless and introduced F_FULLSYNC. They know developers have an incentive to overestimate the importance of their own files at the detriment to system responsiveness and power draw.
What a lot of people really need is just a "provide ordering, don't leave me with inconsistent data" operation. If you lose power "after" a write but before the fsync, for any non-networked application that's no different than losing power before the write, as long as the filesystem doesn't introduce chaos.
I guess I'll add it to the list.
Of course on the other hand, I was already thinking that I should just use SQLite for all my file handling needs. This little nugget makes me think that this was the correct intuition. [Queue horrifying revelations w.r.t. SQLite here.]
It also goes the other way - the update to the directory can be fsynced but the file lost. This can break the "create temp file, write, close, rename to current" scenario (when the intention is to replace file contents atomically).
POSIX doesn't guarantee the order in which data hits the disk, so the above scenario can become "create temp file, write [contents still in memory only], rename to current [written to disk], power failure".
I believe there was a bug where this scenario had worked for a long time then one filesystem (ext4?) pushed closer to what is admissible under POSIX (non-obvious reorders of physical writes) and people started getting random data corruption in programs which used write & rename.
Edit: talking about filesystem misconceptions, not fsyncgate.
Postgres has a whole wiki page [0] about it, it's quite a read. They also link a [1] MySQL commit to fix the same issue.
[0]: https://wiki.postgresql.org/wiki/Fsync_Errors
[1]: https://github.com/mysql/mysql-server/commit/8590c8e12a3374e...
I also found an interesting "scar tissue" from that bug in the current ext4 docs[0]:
"If auto_da_alloc is enabled, ext4 will detect the replace-via-rename and replace-via-truncate patterns and force that any delayed allocation blocks are allocated such that at the next journal commit, in the default data=ordered mode, the data blocks of the new file are forced to disk before the rename() operation is committed. This provides roughly the same level of guarantees as ext3, and avoids the “zero-length” problem that can happen when a system crashes before the delayed allocation blocks are forced to disk."
[0]https://docs.kernel.org/admin-guide/ext4.html
[1] https://thunk.org/tytso/blog/2009/03/12/delayed-allocation-a...
[2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/317781
> POSIX doesn't guarantee the order in which data hits the disk, so the above scenario can become "create temp file, write [contents still in memory only], rename to current [written to disk], power failure".
Wait.. is there a way to do this correctly? At which points is fsync warranted, and how many fsyncs do we need for the whole "write file then mv it on top of current" to not lose data on power failure?
If you truly need to use files you can take other steps such as mv the old file to .bck before mv the new file, but I really think you want sqlite
So while it doesn’t do any other magic, it’s more likely to handle power failures correctly.
Using SQLite3 to avoid power failure issues is pretty good advice. I don't see why GP is getting downvoted.
True.
> Using it to deal with power failures is nonsense.
Using a very well designed library for your use case is not nonsense.
Could you recommend your personal favorite(s) of such libraries? Enquiring minds want to know! Thx.
Of course, the fun part is that the filesystem can’t really guarantee fsync behavior if drives lie about it which many consumer drives do for benchmark reasons. Fun, no?
And if you need this in Java you still have resort to ugly hacks.
We're still living in xkcd 1172 land with this, have been for a decade or who knows how long.
and
"don't believe what close() says"
are two different things.
The article is only (initially) talking about the first, and that is valid.
The second is just second-guessing the OS and hardware environment, and is invalid.
Here's the rule to figure out if you need fsync() or not: "If you think you might need fsync(), you don't." ;)
There are almost no cases where you should worry about the underlying layers. Basically if you aren't writing the filesystem itself, then you shouldn't be calling fsync().
You have to check what open()/close() etc said, but if close() said it worked, then it worked. You're done. The fact that lightning might have struck the drive just exactly then is not your problem and not something you should try to do anything about.
Unreliable networks, busses, batteries, etc none of that changes this. Those things all already have their own layers with their own responsibilities to be doing all the necessary testing and verifying and retrying before they return a success code to you.
There is open(...,O_SYNC) and mount -o sync (udev rules) for the case of a camera or thumb drive connect by usb etc.
It's not merely that you don't have to, it's that it's actively wrong to. fsync() is just joggling someone else's elbow while they're trying to do their job, and if it seems to solve some problem, that actually just exposes that you have some logic or order of operations problem and you aren't doing your own job.
"trust the other layer" or "trust the api contract" is unrelated to and does not conflict with "Be forgiving in your inputs and strict in your outputs.".
It just means:
Do: Check the returned error from, say, malloc().
Don't: Get a success from malloc() and then go try to do things to prove that malloc() actually did work.
That would be insane and impossible because it would have to apply equally to everything, including every single keyword or function you would use as part of the verification. How do you know when you so much as set a value to variable that it actually got set? If you printf the variable to prove it, how do you know printf didn't lie?
The logic for trusting close() is no different from the logic for trusting malloc().
Our responsibility is just to stay within the bounds of defined behavior and not make any assumptions about anything that isn't promised.
You're mixing up the concepts of durability and consistency in pretty significant ways here, and are implying that everybody that is fine with a lack of the former will also be fine with a lack of the latter.
This is absolutely not true, and can cause extremely painful and hard-to-track-down bugs.
For better or worse, there is no way to directly tell the OS "whatever you do, make sure you don't reorder these writes I just did with those other writes I'm about to do".
The next best (portable) thing we have to achieve that outcome is fsync. It's a bit heavy handed, in that it gives you durability even if you only want consistency. That absolutely doesn't mean it's redundant, though.
> Basically if you aren't writing the filesystem itself, then you shouldn't be calling fsync().
Given that fsync is a syscall, but file systems are generally implemented in the kernel, this is a pretty nonsensical statement by itself.
File systems usually have (and need, for performance) a much lower level view of the underlying block storage, and fine-grained control over it.
Just as one example, Linux has the concept of write barriers. Not using these correctly (in the filesystem driver) can cause data leaks across files owned by different users and processes.
Perhaps because there is no reason for such a thing to exist.
Tell me an example.
Databases: People generally don't like unrecoverable consistency errors just because their computer crashed during a write. Not generally possible with reordered writes.
Sometimes people also need durability on top of consistency, e.g. for everything where you want to make at most one request to some server; you can do that by e.g. writing "I did the thing" to a log file, fsync'ing it, and then making your request.
// CheckClose is a utility function used to check the return from
// Close in a defer statement.
func CheckClose(c io.Closer, err *error) {
cerr := c.Close()
if *err == nil {
*err = cerr
}
}
func whatever() (err error) {
f, err := os.Open(blah)
// ...
defer CheckClose(f, &err)
// ...
}
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 ?
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.
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.
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
}
> 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.
If you find the error returned by f.Close to be significant, are you sure returning again it is the right course of action? Most likely you want to do something more meaningful with that state, like retrying the write with an alternate storage device.
Returning the error is giving up, and giving up just because a file didn't close does not make for a very robust system. Not all programs need to be robust, necessarily, but Go is definitely geared towards building systems that are intended to be robust.
Certainly, in the write failure case where there is write failure you'd want to try writing to something else (ideally), notify someone that an operation didn't happen (as a last resort), or something to that effect in order to recover.
But in this case there is no need to try again and nobody really cares. Everything you needed the resources for is already successfully completed. If there is failure when releasing those resources, so what? There is nothing you can do about it.
ulimit -n
You ignore errors on close, and one morning you wake up with your app in CrashLoopBackoff with the final log message "too many files". How do you start debugging this?
Compare the process to the case where you do log errors, and your log is full of "close /mnt/some-terrible-fuse-filesystem/scratch.txt: input/output error". Still baffling of course, but you have some idea where to go next.
After they've fixed what they need to fix you need to use the information now being retained to narrow down why your app is crashing at all. Failing to open a file is expected behaviour. It should not be crashing.
Then maybe you can get around to looking at the close issue. But it's the least of your concerns. You've got way bigger problems to tackle first.
If I recall, Kubernetes performs health checks over HTTP, so presumably your application is using the standard library's http server to provide that? If so, accept is full abstracted away. So, if that's crashing, that's a bug in Go.
Is that for you to debug, or is it best passed on to the Go team?
The bug in this case is in the filesystem that hangs on close. It happens on network filesystems. You can't return the fd to the kernel if your filesystem doesn't let you.
The filesystem hanging is unlikely to be a bug. The filesystems you'd realistically use in conjunction with Kubernetes are pretty heavily tested. More likely it is supposed to hang under whatever conditions has lead that to happen.
And, sure, maybe you'll eventually want to determine why the filesystem has moved into that failure state, but most pressing is that your app is crashing. All that work you put into gracefully handling the failing situation going to waste.
"You wake up and find out that Heroku's staff is anxiously awaiting your departure from your apartment to tell you that your app is down."
That's clearly a bug, and the bug you need to fix first so that you can have your failsafes start working again. You asked where to start and that's the answer, unquestionably.
I really don't know how I can make my explanation simpler.
You literally said that it crashes:
The app crashes because "too many files" includes the fd accept(2) wants to allocate so your app can respond to the health check.
> I really don't know how I can make my explanation simpler.
Not making up some elaborate story that you now are trying to say didn't even happen would be a good start. What you are actually trying to communicate is not complicated at all. It didn't need a story. Not sure what you were thinking when you decided fiction writing was a good idea, but I certainly had fun making fun of you for it! So, at least it was not all for not.
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.
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.
Close just behaves very differently depending on the actual filesystem, too. Usually it’s very fast because it doesn’t do much of anything, but e.g. on NFS close will actually wait for writeback to the server to complete (due to close-to-open semantics).
I think the reality is that most of us push anything where the return status of Close would be important into the database, specifically because it handles semantics like this and simultaneous writes for us. It's like half the selling point of SQLite; you could write JSON documents and handle all the edge cases yourself, or just jam it in SQLite and quite worrying about Close and simultaneous writes and all that junk.
Once you accept this reality, the file case is the same: putting the sync and/or first critical close inline is equivalent work. The issue is that you simply can't -- no matter what the mechanism is -- slip back and forth between your scope maintenance and your error handling monads, resulting in needing cleanup operations where failure is not an option; and, so, you either must not care about the error in the context of the call or must do something even more drastic like terminate the entire program for violating semantics.
FWIW, I do appreciate that people are less likely to make that kind of mistake when working with a database, as people largely get that you should even try to commit a transaction if the code in it had failed somehow. Additionally, I appreciate that if you have a very tight scope -- which Go makes hard, but can still be pulled off -- the "close and throw an error if and only if we don't have an error right now" strategy is not at all horrible... it just isn't a "solution" to the underlying issue without an understanding of why.
Put elsewise, I think it is useful to appreciate that there is more of a universal theoretical / math reason why this is awkward and why it kind of needs to be built in a specific way, and that this issue transcends the syntax or even the implementation details of how you are trying to manage errors: at the end of the end of the day, all of these techniques people discuss are in some sense equivalent, and, at best, most of these workarounds at offer are ways to incorrectly model the problem due to some systems giving you too much rope.
I don't believe people generally care about the error context on the rollback, which makes it safe to defer into a context that can't interact with the error handling monads. Rollbacks shouldn't generally fail, even if they do there's basically nothing you can do about it, and there are few differences between a successful and failed rollback beyond resources on the DB server until the connection is closed.
The Commit is the portion that contains the context people care about in their errors, and that is still safely in a context where it can interact with error handling.
I believe files can get similar atomicity, but it requires doing IO in strange ways. E.g. updating a file isn't atomic, but mv'ing one is. So you can copy the file you want to update into /tmp, update the copy, and then mv the copy to the original file (commit is mv'ing it, rollback is rm'ing it or just ignoring it).
Database transactions aren't atomic and do have the same issue if they reference external resources, though. E.g. if you have a database that stores an index of S3 files, transactions won't save you from writing a file to S3 but then failing to write a record for it into the database. That does muddle the error handling again.
f, err := os.Open(blah)
// ...
defer CheckClose(f, &err)
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.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.
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!
If close fails, I wanna know and I wanna know why.
Never has "who cares" been confidently said. It has always been asked "who cares?". And not asked in a vacuum either, but specifically asked alongside the question of what is to be gained from the knowledge of the error.
We now know that you allegedly care, which is a promising start. But you purposefully ignored the other question, which questions the credibly of your care. You can't meaningfully care about something if you don't know why you care about it, and if you knew you'd have told us about it already as it nonsensical to answer to the "who cares?" question alone, so...
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))
}
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
}`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.)
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.
Can you give an example case of how this could happen?
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
}
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
}
}
Then you can use defer in the file-writing function if you so please, and not bother to close at the end explicitly, without issue. A more robust example might be to even include the sync call in the deferred function (and even clean up the file itself on error). To re-use your example from your blog post:.
func helloNotes() (err error) {
var f *os.File
f, err = os.Create("/home/joeshaw/notes.txt")
if err != nil {
return
}
defer func() {
if err == nil {
err = f.Sync()
}
cerr := f.Close()
if err == nil {
err = cerr
}
if err != nil {
os.Remove("/home/joeshaw/notes.txt")
}
}()
err = io.WriteString(f, "hello world")
return
}
defer SafeClose(f, &err)
FWIW, I'm not sure why you are so negative on named return values, but I'm at best a novice Go programmer, so perhaps I don't fully understand why they aren't great (I guess it does look weird to me to have bare `return` statements that do actually return a value even though it doesn't look like it). Your argument about the return value possibly being modified after the core function finishes being unintuitive doesn't really strike me as a big deal either.
f, err = with(os.Open(thing)) {
// do stuff
}
// at this point err has the same semantics as your CheckCloseGenerally, relying on defer in Go or Drop in Rust for anything that can fail seems like an anti-pattern to me.
0: https://doc.rust-lang.org/std/fs/struct.File.html#method.syn...
I assume a big issue is that this is full of edge cases up the ass, and the value is somewhat limited in the sense that if you know you want durable writes you'll sync() and know you're fucked if you get an error, but close() does not guarantee a sync to disk, as the linux man page indicates:
> A successful close does not guarantee that the data has been successfully saved to disk, as the kernel uses the buffer cache to defer writes.
So you'd need a "close", and a "close_sync", and possibly also a "close_datasync" (if you're ok with discarding metadata). And one could argue at that point `close` has essentially no value beyond hopefully getting rid of the fd / handle, and drop already does a fine job of that.
What we really need is a way to handle effects in drop; one way to achieve that is to have the option to return Result in a drop, and if you do this then you need to handle errors at every point you drop such a variable, or the code won't compile. (This also solves the async drop issue: you would be forced to await the drop handling, or the code wouldn't compile)
Is it though? It ensures the fd is closed which is what you want, and if you have some form of unwinding in the language you can't really ask for more. And aborts are, if anything, worse.
It also works perfectly well for reading, there's no value to close errors then.
> Closing a file can raise errors but you can't reasonably treat errors on drop.
It's mostly useless anyway, since close does not guarantee that the data has been durably saved. If you want to know that, you need to sync the file, and in that case errors on close are mostly a waste of time:
- if you've opened the file for reading you don't care (errors are not actionable, since you can't retry closing on error)
- if you've flushed a write, you don't care (for the same reason as above)
The one case where it matters is if you care but missed it, in which case we'd need a bunch of different things:
- a version of must_use for implicit drops
- a consuming flush-and-close method on writeable files
- a separate type for readable and writeable files in order to hook both, and a suite of functions to convert back and forth because even if rust had the subtyping for you don't want to move from write to read without either flushing or explicitly opting out of it as you're moving into a "implicit drop is normal" regime
> one way to achieve that is to have the option to return Result in a drop, and if you do this then you need to handle errors at every point you drop such a variable
That is nonsensical, the entire point of drop is that it's a hook into default / implicit behaviour. How do you "handle errors" when a drop is called during a panic? It also doesn't make sense from the simple consideration that you can get drops in completely drop-unaware code.
Consuming methods is what you're looking for.
struct Foo {
bool dirty,
}
impl Foo {
fn clean_up(&mut self) {
// ...
self.dirty = false;
}
}
impl Drop for Foo {
fn drop(&mut self) {
if self.dirty {
panic!("Foo was not cleaned up before its lifetime ended");
}
}
}
fn bar(foo: &mut foo) {
foo.clean_up();
}Yes, I call this "RAII is a lie" (T-shirt pending).
Closing file descriptors is univerally used to showcase RAII, but it should never be used for that.
C++ has the same problem:
https://github.com/isocpp/CppCoreGuidelines/issues/2203
In there, it is acknowledged that a manual Close() should always be provided, and used if you want guarantees.
> is a bad pattern
Good that Rust at least figured it out early that it's a bad pattern!
Never use RAII in situations where the cleanup can fail!
The tasks seem simple from 30,000 feet up in the air. Once you get down into the dirt you realize there's absolutely nothing simple about what you're proposing.
A filesystem is a giant shared data structure with several contractual requirements and zero guarantees. That people think a programming language could "solve" this is what is boggling to me.
This is a classic safety / performance trade off that was properly selected in favor of performance.
The defer Close() is still quite useful as a way to avoid fd leaks.
You'd just concentrate on the "happy path", you'd close the file, there'd be nothing to forget or write blog posts about because the exception would be propagated, without needing to write any lines of code.
Both have advantages and disadvantages, I’d say the more “modern” approach actually the opposite to what you state here, and is in my opinion the way to Go (pun int intended), though it’s also how Haskell does it. You’ll find the same philosophy in Rust, Zig, Swift and others which all build on the previous decades of throwing exceptions and how terribly that scales in terms of maintainability. Even in the “old world” like with Java you have Kotlin which does both.
(I might misunderstand your use of "though", though? It could be that you were just noting in passing how Haskell disagrees with all of these supposedly-"modern" languages, and instead leaned into the sane happy path semantics, thanks to monads.)
(edit: to be clear, though... I do not think exceptions solve this. I wrote a comment elsewhere on this thread about the semantics issue, but a few other people also wrote similar things while I was trying to type my overly-verbose reply ;P.)
Yeah, it is - a bad choice IMO.
I know the "if err != nil" pattern is spoken up as some sort of cultural idiosyncrasy of Go, similar to the whitespace formatting in python. But so far, I haven't seen any actual data (or even arguments) why it is superior to exceptions, or which inherent problems of exceptions it solves. (The classical example of "it makes control flow more obvious and makes it easier to ensure that mandatory finalizers are not skipped" was just disproven by this very article)
So if there is more substantial criticism against exceptions than just FUD, I'd like to know it.
defer func() {
if r := recover(); r != nil {
fmt.Println("Failed to write file", r)
}
}()
f := os.Create("file")
defer f.Close()
io.WriteString(f, "Hello, World!")
So clearly exception handlers aren't enough. There might be some good ideas in using exception handling, but if you strictly limit its use to errors you end up half-assing it. Why not go all the way and find a solution that works in all cases?
If you focus on errors, you’re going to never get it. The whole idea here is that the problem is bigger than errors.
Do you mean the issue that if both WriteString() and Close() throw an exception, the one from WriteString() will be swallowed?
That used to be a problem, but has been solved in more modern implementations using suppressed exception tracking.
Think more carefully about what we're actually trying to solve. It is not just about errors.
It's faster. Doing all error handling via exception is not viable if you want speed.
Exceptions work well for errors in the sense that these rarely happen. But using them for general "this is the bad outcome" of an operation that can happen in the hot-path is problematic. For example pythons "next()" function which raises an exception if the end of the iterator is reached, if C++ did this it just wouldn't be used.
So in my opinion exceptions are nice if you want ease of use, and explicit error handling (look at rust with their question mark operation which makes it pretty easy to do) is the way to go for performance.
One advantage of explicit error handling like rust does it is: it forces you to handle the error path like other business logic. Again, if you write a short script, this is annoying and doesn't get you much but if the application is very important then doing so is a good thing since it forces you to think about where which errors can happen and how you handle them. With exceptions its very easy to completely forget that a can even happen and thus they get ignored and suddenly the program crashes with a unreadable error message.
(As for "handling" errors, you should only have a few places in the entire codebase which do that... littering the entire codebase with opportunities to feel like you might could handle an error seems like a mistake as it just encourages more handling.)
The Go designers have explained and talked about this decision many times. Some people don’t like it and maybe choose a different language. No one pushes FUD regarding exceptions. I get the impression that if you choose a Toyota you would think that your neighbor buying a Honda poses a criticism you have to address.
Discussed before many times on HN, such as:
https://news.ycombinator.com/item?id=4159641
Original article:
https://commandcenter.blogspot.com/2012/06/less-is-exponenti...
If under ‘every approach’ you explicitly exclude go’s terrible errno syntax sugar, and include exceptions and sum types, then yeah. There is zero advantage to go’s error handling compared to the proper sum typed solutions.
Go's designers have explained their decisions many times, I won't repeat their justifications here. Obviously they had to make choices in line with their overall vision for Go. They feel strongly that programmers should handle or at least acknowledge the possibility of every error, explicitly, right where it gets reported. Exceptions and sum types don't enforce that.
Then consider that if any code you're calling is not exception-safe it makes the task of writing your code to be exception safe that much harder.
Then add on top of that the question of responsibility -- do you handle the exception close to where it gets thrown or farther up the call stack? Handling it too close to the issue may be missing some context and result in a lot of the same exception handling across many areas of the codebase. But handling it too far up misses context, too, and often leaves the program in an uncertain state where it's not clear whether the show must go on or if it's time to shut it down. It's not uncommon to see a mix of exceptions and returned error values to try and find a happy middle ground there.
Java tried to paper over some of these problems with fewer gotchas (more explicit separation between resource allocation and initialization) and compiler checks that exceptions are in the method signature and are always handled _somewhere_ up the call stack but this often results in handling of very vague exception types at a high level. Some developers, uncertain what the right way to handle an exception is, and not wanting to crash the program, will just silently drop the exception instead! Admittedly, you can do this in go-style and C-style error handling, too, but at least then it's not as far removed from the source of an error because it's annoying to have to keep passing an error response through so many function signatures.
I used to use exceptions a lot in C++ and Java. This changed when I started working at a place where a lot of C++ is used but without exceptions. It was ostensibly about runtime costs but when seniors were pressed on the issue it was clear there were a lot of these other reasons stated above, and ultimately about readability of the code (error handling being close to error source) and a philosophy of failing quickly when assertions fail (because keeping calm and carrying on leads to programs running that should have died before they could make more of a mess).
I know it's unpopular but, in light of some real problems with exceptions, I actually prefer the way Go makes you do it. It often encourages doing all the setup in one (or a few) function scopes, and it becomes pretty evident from a function's signature whether you should expect something to go wrong or not. Would I prefer it for a game-dev scripting language? no! But for a system language it is better IMHO
It’s not like exception handling and throwing around things to have them caught later is inherently bad. It’s just a different philosophy, one that I don’t personally like anymore. It’s down the same alley as things like OOP, SOLID or DRY. Things which have good concepts that way too often leads to code bases which are incredibly annoying to work with. Maybe not for small systems with short life times, but for systems where you’re going to be the 100th person working on something that’s been running for 30 years it’s just nice to not have to play detective. I’d like to put a little disclaimer in here, because that isn’t inherently a consequence of exception handling or any of the other concepts but it’s just what happens when people work on code on a Thursday afternoon after a tough week. The simpler less abstract things are made, the easier it’ll be to unravel, and simple error handling is dealing with the errors exactly where they occur.
As others point out, it’s not without its disadvantages. It’s just that in my experience, those are better disadvantages than the disadvantages of implicit error handling.
OK, here's an argument.
- In order to write resilient software, programs must handle not only the "happy path" when things succeed, but the path where things might fail.
- Thus it is important for developers to 1) be aware of which operations may fail fail, and b) think about what the program should do in that case.
- Exceptions make it easier for the programmer to forget that something might fail, and to avoid thinking about what to do if it does fail.
- Go's error handling idiom makes it clear that an operation might fail, and prompts programmers to think about what to do in that case. (They may of course choose not to think about it, but at least they made a conscious choice at some level.)
Thus Go's error handling idiom nudges developers towards more resilient software than exception-based workflows.
Or to put it differently: Programming systems which may fail simply is ugly: there are an exponential number of ways a system may fail, and each one must be handled correctly. Exceptions hide this ugliness, but by doing so make it more likely that there will be cases not handled correctly. By exposing this ugliness, Go makes it more likely that most cases will be handled correctly.
And exceptions let you handle error conditions without making the actual business logic harder to read, with as little or much specificity as required.
> Thus it is important for developers to 1) be aware of which operations may fail fail, and b) think about what the program should do in that case
Checked exceptions/effect types exist, being explicit or implicit in function signatures is not a fundamental property of exceptions.
And what is clearer in terms of error handling — if err being every third line, with questionable handling logic, e.g. just printing or swallowing stuff (or gestures at the article), and definite human error from repetition —— or a well-defined block with proper scoping, without which the error case does the only reasonable thing — automatically bubbles up, making it possible to handle higher up. There is often no immediate action that can be done in certain exceptional situations, e.g. your ordinary function that writes a file can’t do anything about a full disc. The best it can do is to yell, so that the action that called it somewhere can do some evasive action, e.g. re-trying/notifying the user/etc.
> Exceptions make it easier for the programmer to forget that something might fail, and to avoid thinking about what to do if it does fail.
Disagree. If anything, something not being in a try-catch block says that it will be handled higher up (or checked exceptions making it part of the signature), and when it’s surrounded by it, I know what is the happy path, and unhappy path immediately, without it being crossed over (usually badly), as it would happen with if errs.
> Go's error handling idiom makes it clear that an operation might fail
What about the case when it both returns a value and an error?
> and prompts programmers to think about what to do in that case
Blindly if erring and printing out a random string is not error handling. That’s just noise, and a terrible trap for yourself, having to grep for useless error codes later on.
I don't think you get what I'm saying. Some functions will always succeed. Some functions fail in obvious ways. Some functions fail in non-obvious ways. How do you know, as you're scanning a long block of code, which operations may fail, and which will always succeed?
For instance, suppose you have code like the following:
// Decode they key JsonKeyGuids as type []Guid
guids := JsonGetKey[[]Guid](&ru[i].Json, JsonKeyGuids)
And while checked exceptions might help, it's not perfect: Suppose your code block calls functions a(), b(), and c(); all of them return ErrParseFail, but while it's pretty obvious that a() or c() might fail that way, it's not at all obvious that b() would.
Secondly, even for operations that are obvious may fail: maybe you, as a senior programmer who has programmed with exceptions for years, are paranoid enough that you're always thinking in the back of your mind "what happens if this fails?" But I very much doubt a junior programmer is going to have that habit. Part of the intent of Go was to have a language for Google which would allow junior programmers could come to a previously unfamiliar bit of code and be reasonably effective very quickly.
> And what is clearer in terms of error handling — if err being every third line, with questionable handling logic, e.g. just printing or swallowing stuff (or gestures at the article), and definite human error from repetition
You'd almost never just print the result of error messages unless it's at the top level, or it's the equivalent of a script. In most cases, you bubble it up, often wrapping it with a message of what you were trying to do; e.g.:
return nil, fmt.Errorf("trying to excluded guid list for %v: %w", ru[i].Id, err)
Not having the equivalent of C's "must_check" is certainly a missing guard-rail in golang
> And what is clearer in terms of error handling...
It comes down to a judgement call. I think Golang's way is better. Yes, it makes the code look cluttered with exit paths, but that's because the code is cluttered with exit paths.
I can see that with experience, an exception-based developer would learn to see the implicit exit paths in most cases. So let me assert to you, that with experience, a check-the-return-values based developer also learns to filter out the explicit error paths to see the "happy path" algorithm clearly. But on the whole, I think the latter is likely to lead to fewer bugs, particularly for less experienced developers, but even for more experienced developers.
At any rate, now you've heard arguments for Go's error handling idiom; and if you don't agree, at least you can understand where the Golang crowd are coming from.
It is literally C’s shitty `errno` with syntactic sugar.
Go's error returns are not sum types, but they are product types. The return signature (T, error) indicates that two values will be returned essentially as a tuple by the function: one of type T and one of type error. Error-returning functions are pure functions (though they typically perform other, impure operations).
There is no syntactic sugar (both for good and for ill). The type of errors is an ordinary interface, with a single method. Any type can implement that interface, including strings and structs and slices. Errors can have as many contextual details as needed, including nested/wrapped error messages, specific parameters of loop iterations, multiple errors rolled up from multiple operations, etc.
Go's error return is just an ordinary but common use of its multiple-value returns. You can write a function/method that returns three ints and no errors:
func (v Vector3) Splay() (x int, y int, z int)
func DoThisAndThat() (thisError error, thatError error)
Technically, yes, practically, it doesn’t tell you anything, as its most common usage is how it would be used as a sum type (either one or the other).
And yeah, I didn’t quite think of multiple return values, but that itself can be just syntactic sugar over an `out` parameter.
But these technical details aside, I am not convinced that it is not “as useless as errno-type error handling”, with the only caveat of it returning an interface that is slightly more informative.
Of course, nothing stops you from building your own file handing package that overloads exception handlers to deal with errors. If it gains traction then it would prove the stdlib should consider a v2 API.
But that you already haven’t done so is telling…
Anyway, most don't. There is no difference from exception handlers in most other languages.
The syntax is a little different. Is that where you've become confused?
Are there actually ANY languages other than go that have coroutines, and try/catch/throw mechanisms, where you cannot throw across a coroutine boundary?
And why would exception handlers NOT work across coroutine boundaries, other than laziness on the part of implementers?
You wha...? The question was about goroutines, not coroutines.
Besides, you'll notice that exception handlers cross coroutine boundaries in Go just fine. Your random tangent isn't even correct. Where did you dream up this idea to the contrary? I know coroutines are still new to Go, only officially landing in the latest release (experimentally in 1.22), but you'd think that would also mean their behaviour is fresh in your memory.
I'll take your avoidance of the original question to mean that no other language does it either.
What are goroutines, other than peculiarly broken coroutines? (Notwithstanding your point that go has a non-broken implementation of coroutines at experimental release stage).
It is true that Javascript has a goroutine-like facility for executing coroutines on a seperate thread. But there are languages (c++, c# at least) where coroutines can execute on separate threads without suffering from the broken-ness of goroutines.
> If it gains traction then it would prove the stdlib should consider a v2 API.
Some library that behaves completely differently from the rest of the language and breaks all interop with the rest of the ecosystem will have a hard time gaining traction, no matter if the way the library does it is objectively better or not.
Probably for the same reason Rust does, and why it suffers much the same problem:
1. It is what was in vogue in the 2010s.
2. More importantly, the problem isn't limited to errors. What have you gained treating errors as some hyper special case when they aren't any different than any other value?
I think we agree that we can do better, but seeing errors as special doesn't get you there. We need something that understands the all-encompassing problem.
So, failing that understanding, if you're going to do something that sucks, you may as well choose the least-sucky option, surely? Exception handling brings a horrible developer experience. To the point that in languages where errors over exception handling semantics are the norm, you will find that most developers simply give up on error handling entirely because it is so painful.
> Some library that behaves completely differently from the rest of the language and breaks all interop with the rest of the ecosystem will have a hard time gaining traction
I'm not sure history agrees. Ruby was also of the return values over exception handling mind before Rails came along. Rails pushed exception handlers for errors and developers went for it. Provide an API people actually want to use, and they'll use it. What was common before is inconsequential.
I expect what you are really saying is that exception handling wouldn't actually improve this example case even in the best case, and in the worst case developers would end up giving up on error handling leaving such a package to be a net negative to a codebase.
So it is telling. But I think what it actually tells is that people would have done it just use another language instead.
Finalizers shouldn't fail because they might be executed while another exception is already in flight. Three languages have three different behaviors when that happens but in my opinion they all do the wrong thing:
In C++, if you throw from a destructor while another exception is in flight, the program will be terminated. In Java, throwing from a `finally` block will "forget" the original exception. In Go (according to this article, I'm not familiar with it), error from `defer` will be ignored. None of these are ideal.
What would be the right thing? Combining the original exception and the error from `close` into some kind of `MultipleError`?
I don't thing there's one true right thing™ though. That's why explicit handling is necessary: The compiler doesn't have enough context to handle it for you. The programmer needs to decide what's the right way to handle it.
Furthermore in Java since version 7 you can actually see both exceptions with the suppressed exceptions pattern.
Since function calls form a tree, exceptions must form a tree as well.
Doing this automatically is also one of the killer arguments for exceptions over error codes, IMO.
When I say "exceptions", I mean the source-level semantics, not how it's implemented behind the scenes.
> Doing this automatically is also one of the killer arguments for exceptions over error codes, IMO.
Definitely doing this automatically is better than relying on the programmer to do this manually, but I wouldn't say this is the "killer" argument for exceptions over errors codes, because this doesn't add anything new to the argument of exceptions vs explicit error handling.
try {
using var file = File.OpenWrite("test.txt");
file.Write("Hello, World!"u8);
}
catch (IOException e) {
Console.WriteLine(e.Message);
}
This includes an exception when trying to open or write to the file. If I fail to write to a file, it will try to dispose the stream, which flushes it and closes the file handle. If disposing the file handle itself fails, which should never happen, the exception will occur in the finally block, which this exception handler catches too. If you need to disambiguate and handle each case differently, which is rarely needed, you can order try-catch-finally blocks differently with explicit dispose and different nesting. This, again, is not a practical scenario and most user code will just `using file = File.OpenWrite` it and let the exception bubble up.
Just simply not explicitly handling every single possible error is the correct choice in many scenarios - in which case it bubbles up to a general error handler, e.g. telling the user that something bad happened here and here.
>>> mylist = []
>>> try:
... first = mylist[0]
... finally:
... inverse_length = 1.0 / len(mylist) # imagine this was something more complex
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
IndexError: list index out of range
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 4, in <module>
ZeroDivisionError: float division by zero class A:
def __del__(self):
1/0
Or monads, but that might be a step too far for the Go world considering the push-back against generics. When you have declarative error handling (a good thing imho) then monads really are the bees knees.
The answer here is that everything throws.
Any code can have a Null/Nil dereference error, any code can use an array and generate an out-of-bounds exception, etc.
You do in Java. It's called Checked Exceptions. It's a binding API contract and communicates this well not only when writing the code, but also when reviewing it.
Another thing is that if you're holding an flock on that file, it's nice that closing it will drop the lock. Generally you want to drop the lock as soon as you're done writing to the file. Deferring the dropping of the lock to the deferred close might cause the lock to be held longer than needed and hold back other processes/threads -- this doesn't really apply in TFA's example since you're returning when done writing, but in other cases it could be a problem.
Do not risk closing twice if the interface does not specifically allow it! Doing so is a real good way to create hard-to-find corruption bugs. In general if you see `EBADF` when closing any fd values other than `-1`, that's a really good clue that there is a serious bug in that code.
defer func() {
cerr := f.Close()
if err == nil {
err = cerr
}
}()
defer func() {
cerr := f.Close()
err = errors.Join(err, cerr)
}()You could write your own errorsJoin() and change Error() method to suit your needs.
But really in this particular scenario you would be better served by something like:
func errorsConcat(err1 error, err2 error) error {
if (err1) {
return err1;
}
return err2;
}
In the scenario described in this article, errors.Join() would most often reduce to that (in terms of what Error() string would produce).
I miss exceptions
1 - https://go.googlesource.com/proposal/+/master/design/go2draf...
That said, a language feature where you can throw lightweight error values without generating a stack trace etc might be a middle ground. But it won't land in Go, given the discussion about alternative error handling some years ago.
Anyway, in practice it's not that bad. A write can go wrong, you as a developer should write code that handles that situation. Exceptions lead a developer to miss errors, or to not handle them in a finegrained manner - e.g. generic "catch all, log error, maybe" style error handling.
> Exceptions lead a developer to miss errors, or to not handle them in a finegrained manner - e.g. generic "catch all, log error, maybe" style error handling.
I don't see how Go error handling makes people handle things any more explicitly than exceptions. Most people just `if err != nil { return err }`, which to be honest is the _correct_ logic in many cases, and it's pretty easy to forget to check if err and keep on trucking. At least with exceptions if you don't catch it your thread terminates making unhandled exceptions
Exception bubbling means its easier to catch the error at the level that makes sense, and because they are real objects type checking is easier as opposed to the performance of `errors.Is()` which is surprisingly slow.
It is almost never the correct logic. The only time it might be appropriate is in a private helper function that has limited scope around another function.
It is most definitely not the correct logic if you are returning that from a public function! For many reasons, but especially because it now binds you to the implementation of the function you called forevermore. That is a horrible place to be.
For example, find out your os.File usage would be better served by SQLite? Too bad. You can't change it now because the users of your function have come to rely on errors from the os file operations when they handle the error you give them. Their code can't deal with the errors coming out of SQLite.
Instead, you need to return errors that are relevant to your function. It may be appropriate to wrap the source error in some circumstances, but your error structures should compel the user to rely on your errors, leaving the wrapped error only for things like logging where a change in the future won't break the callers.
The core issue here is that you want to deal with errors as soon as possible. The async nature of writes to files makes this more challenging. Part of the purpose of this post was to inform people that you don't necessarily always see write errors at write-time, sometimes you see them at close-time and should handle them there. (And sometimes you don't see them at all without fsync.)
Even if you do handle errors in your deferred close, you may be taking other actions that you'd rather not have until the file i/o is completed, and this could leave your program in an inconsistent state. Side effects in Go are common, so this is a practical concern and it is hard to spot and debug.
[0]: https://github.com/ziglang/zig/blob/fb0028a0d7b43a2a5dd05f07...
It may often be better to handle the issue as a system failure with fanotify. https://docs.kernel.org/admin-guide/filesystem-monitoring.ht...
> On modern hardware if a disk is throwing an io error on write you are having a bad day.
And how would you know you're having a bad day if apps ignore those errors?
Whether you handle the error or immediately or if you allow the error to occur after a defer, you still are almost certainly not handling it properly and are taking a speed hit for your troubles.
This is really all normal use cases under normal conditions.
Whether if it's allowed to write again after a "commit pending writes" operation or not is a separate design decision. If not, then in some languages it can be expressed as an ownership taking operation that still allows for handling the error.
I wrote my own version for $WORK package (as our policy is that all errors must be wrapped with a message), used like:
func foo() (retErr error) {
x := thing.Open(name)
defer errors.Close(&retErr, x, "close thing %v", name)
...
}
Finally, errcheck can detect errors-ignored-on-defer now, though not when run with golangci-lint for whatever reason. I switched to nogo a while ago and it found all the missed closes in our codebase, which were straightforward to fix. (Yes, I wish the linter auto-ignored cases where files opened for read had their errors ignored. It doesn't, so we just check them.)
Multi-errors are nice.
Like this?
switch {
case strings.Contains(err.Error(), "close thing foo"):
// deal with foo close error
case strings.Contains(err.Error(), "close thing bar"):
// deal with bar close error
}
Like, are you doing anything with TLS? String matching: https://github.com/golang/go/issues/35234
Using the stdlib ssh stuff? String matching: https://github.com/golang/go/issues/45207 / https://github.com/golang/go/issues/39259
Want to parse an address + port? netip.ParseAddrPort only returns strings ('errors.New' errors).
http/http2 is also a minefield of half-exported errors.
The go authors say to use 'errors.Is' and 'errors.As', but the go stdlib also defines an idiom, and the idiom it defines is that somewhere around 30% of all errors should be stringly typed, including many where you may want to have specific handling for them.
Realistically, you can't follow the standard library, except perhaps the newest additions. Idioms emerge and evolve with use. Much of the standard library was written before Go saw much use, being largely in place before the world got to see Go for the first time. Also, thanks to the Go1 guarantee, cannot be changed now.
If the aforementioned package was written in the 2000s, then it might be fair to say that it was in line with the idioms of the time. But it appears to have been written within the last year, and thus is not aligned with idioms of its age.
That's not to say it has to be. Idioms are not requirements. The "stringly-typed" design may be justified even knowing what we know in the 2020s. And, with that, we don't need to speculate about the justifications. We can let the author speak for himself as to why the choice was made.
I always interpreted the preference for stringly typed errors as a way to keep the Go language simpler. Good error handling is complicated and hard to read, and one of Go's values is that programs should be easy to read. As such, if you want good error handling, you should use a different language, like Java or Haskell or C++. This also helps keep people who might demand complicated things like generics away from the language, further keeping it simple.
My understanding was that many of the go idioms are there to scare off programming language theorists, who have a tendency to unnecessarily complicate everything with type theory, and error handling also seems to mostly be in that vein.
Not exactly. It assumes that all error conditions within the functions provided by netip are of the same nature as it pertains to a single unit of work. In other words, there is only one type (not referring to the language's type system). Error type reuse where different failure points produce the same type of error does not violate current idioms. I cannot immediately think of any reason for why their assumption is wrong, so unless you have other ideas?
That is not the same situation as the deferred close wrapper, though. It is assuming that closing multiple file handles is the same operation, but clearly that's not true. If you were, say, writing a copy function the error handling of the read handle failure is unlikely to be the same as the handling of the write handle failure. The former doesn't tell you much, the latter is quite actionable. The failure points are distinct, and thus of different types (again, not referring to the type system).
The author's answer was basically that he is the only caller so if that problem arises in his code he'll simply modify the function to return idiomatic types. Which is fair for the lone wolf developer. When working alone anything goes! But it is not good API design generally speaking. It certainly wouldn't fly in something like the standard library or anywhere you have other developers.
I have a program that takes user input and parses it, and then displays an error. My program is for a language other than english so having it display a pop up with the message "invalid ip:port, square brackets can only be used with ipv6 addresses" in english is bad. Therefore I want to switch on the error message to display translated errors, but of course Go does not think that parsing errors are something that is important.
If parsing user input isn't a place to expose clear non-stringly-typed-errors, I don't know what is.
Note, it also gives bad errors in that some of them include details and the user input, and some don't, so displaying them to users will stutter or require parsing.
For example:
_, err = netip.ParseAddrPort("foo:bar")
// invalid port "bar" parsing "foo:bar"
_, err = netip.ParseAddrPort("1.2.3.4:")
// no port
I know the answer to all my issues is the usual go thing of "a little copying is better than depending on the go stdlib" of course. At least for netip, forking it is fine, having to maintain a fork of the go net/http stack just to get halfway decent errors is a real pain.
Imagine a function that looks like:
addr, err := netip.ParseAddrPort(input)
if err != nil { return err }
err = makeConnection(addr)
if err != nil { return err }
The way to do that is to have `netip.ParseAddrPort` failures return a different error type than other methods, but the "idiomatic" go code above doesn't wrap the error with an additional type, so the caller can't distinguish between a network error (many of which are also just 'errors.New'), and a netip parse error (all of which are 'errors.New').
Pushing that responsibility onto the callers seems silly, and like a footgun, especially when several other packages do have typed errors that mean the caller can successfully identify the error without having to do verbose explicit wrapping.
No. Absolutely not. If you leave the callers of this hypothetical function to rely on the errors of the functions it calls, even if we assume those functions were written by an infallible deity, you've created a coupling that now binds you to the implementation forevermore. That's just plain reckless behaviour.
Return your own errors. I know it takes a tiny amount of extra thinking to figure out what types are relevant to your function, but is unquestionably worthwhile and would still be worthwhile even if all the functions you call were designed by an infallible entity.
> but the "idiomatic" go code
You're really stretching the use of idiomatic here. Not ever has that been considered idiomatic Go. The Go community has always been clear that you should never, ever write code like that. It is so painfully horrid for so many reasons that it could never be considered idiomatic.
If I'm going to do something upstream based on the error, I use a type. Most of the time, it's just log.Error text though.
That may get you through if there is only one x.Close, but now you have to offer the guarantee to the callers that there will only ever be one Close error returned. Furthermore, any callers of your function have to ensure that they don't end up introducing additional Close error returns in the same vein.
Without such guarantees, you, the caller, need to resort to string matching to protect against undocumented functionality and/or future modifications when you handle the error. Sounds like a rough situation...
> If I'm going to do something upstream based on the error, I use a type.
Implying that you are a lone wolf developer? I think you make a good point that if you exist in your own world without other developers just about anything goes.
That said, the language used around the previously linked repository implies that it welcomes other developers using and working on the code, so it is not clear how you are "doing the upstream" in all cases.
No you don't. It's a multierror, a feature of the standard library.
errors.Is(
errors.Join(ErrA, ErrB, ErrC, errors.Join(ErrD, ErrE, ErrF)),
ErrF)
> Implying that you are a lone wolf developer?
You know that's not true from the history of the repository. We do have coding standards that reviewers enforce, this is one of them.
Understood (I read the code), but that doesn't help, and really has nothing to do with the topic at hand. Is the problem here that you don't have an understanding of what we're talking about?
> You know that's not true from the history of the repository.
Which is why the claim was identified as being strange and in need of clarification. How can "If I'm going to do something upstream based on the error, I use a type." be true? If there are many developers, you don't get to control the upstream.
Which operating system did you experience this under and was it the operating system, your Libc or what else in the stack which caused this?
Then code acting on the stale handle of the first file closes it, and accidentally closes the new file instead.
That is a scary sounding error though.
How does the Python/C# approach compare to Go's in this situation? How are errors in `__exit__`/`Dispose` handled?
https://docs.python.org/3/library/stdtypes.html#contextmanag...
The comment timestamps are temporarily adjusted to be consistent with the adjusted posting times, such that readers aren't confused by the comments predating the apparent posting time.
The timestamps will revert back to their original values in a few days.
What I don't understand is why this went into the second chance pool if the original submission made it to the front page and got >100 points.
More details: https://news.ycombinator.com/item?id=26998308
If I write to file on a reasonably recent Linux and a sane file system like ext or zfs, at which point do I have the guarantee that when I read the same file back, that it is consistent and complete?
Do I really need to fsync or is Linux smart enough to give me back the buffer cache? Does it make a difference if reader and and writer are in the same thread or same process?
It’s more complicated if the computer shut down in between, depending on how clean the shutdown was.
This is also how I'd solve it in Rust, except the defer would be implicit.
func doSomething() error {
f, err := os.Create("foo")
if err != nil { return err }
defer func(){ _ = f.Close() }()
if _, err := f.Write([]byte("bar"); err != nil { return err }
return f.Close()
} defer f.Close()
defer func(){ _ = f.Close() }()
> Should you have a linter catching these things?
JetBrains’ GoLand will in fact warn you of this. If the error truly is immaterial you can instead do
defer func() { _ = f.Close() }()
which is verbose but explicit in its intent to ignore the error.
Ahhhh okay I see it now. I definitely prefer to not use that feature as well, and I’m surprised it’s even there given how well the rest of the language adheres to “only one way to do things”. Doubly agree that it’s a strange “hack” for forwarding the deferred return value… oof
> JetBrains’ GoLand will in fact warn you of this
Heh yeah that’s what prompted me to ask, as I noticed (and very much appreciated) these hints. 100% agree with the verbose-but-explicit example you gave, and do that myself.
func doSomething() (err error) {
var f *os.File
f, err = os.Create("foo")
if err != nil {
return
}
defer func(){
if nil != f {
f.Close()
}
}()
_, err = f.Write([]byte("bar")
if err != nil {
return
}
err = f.Close()
f = nil
return
}
if err := f.Close(); err != nil { return err } return nil
Is equivalent to
return f.Close()
https://pkg.go.dev/os#File.Close
“Close will return an error if it has already been called.”
An os.File is a data structure containing a file descriptor and some other fields. It is safe to call Close() multiple times, because it will only call the underlying syscall close() once.
Causes include memory failure, drive cable melted, network cable pulled, etc.
What to do?
How important is the data being written? Is the only copy of just aquired data from a $10 million day geophysical survey? How much time and resources can you spend on work arounds, multiple copies, alternative storage paths, etc.
In aquisition you flush often, worst case lose a minute rather than a day.
In, say, seismic quisition, you might aquire audio data from microphone array and multi track raw audio to SEGY tape banks AND split raw data to thermal plotter AND processing WHERE RAW DATA -> (digitally to DAT AND hard drives) and through processing WHERE COOKED DATA -> digital storage.
In processing pipelines a failed write() or close() isn't so bad, you flag that it happened and you can try to repipe the raw data to get a savable second result.
Ultimately you want human operator control on what and when to do something - it's a hardware problem or resource starvation at the root.
Other than the recording redundancies described (raw analog logged, raw digital logged, raw paper chart created, cooked data logged, cooked paper chart, (raw | cooked each on tape, disk, paper) what are these "redundant systems" that you speak of?
Keeping in mind, of course, that the client has raw data, etc. on the contract as deliverables.
Do you imagine two full ships pulling two full microphone arrays to offset a rare (but happens) recoring failure? Now you've doubled the per dium costs and halved the area that can be covered in a typically short season.
Do you imagine one ship pulling two arrays that magically don't tangle? It doesn't work that way.
The goal here, of course, is to do all that as feasibly possible upfront in order to minimise aquisition time on the water and to ensure that all pings | booms | etc and their returns to multiple mic's recorded so the ship doesn't have to do a repass.
Expand on your non cargo culting non insane design ideas for 1970-1990s offshore seismic exploration by all means as what you intend isn't clear in your terse comment.
Keep in mind your design will need to be moved on and off arbitrary ships and will operate in places like the North Sea, Spratly Islands, etc. and will have to survive the pitch and toss of stormy weather (eg: attention to card fit in bus backbone).
func f(filename string) error {
fp, err := os.Create(filename)
if err != nil {
return err
}
defer fp.Close()
if _, err := fmt.Fprintln(fp, "Hello, world!"); err != nil {
return err
}
return fp.Close() // safe to call multiple times
}
On Linux, the close() system call rarely fails unless you provide an invalid file descriptor. L.Torvalds has stated that on Linux, close() immediately removes the file descriptor from the process, regardless of the underlying implementation's success or failure. Any errors related to the actual closing of the file are handled within the kernel and won't affect user-space programs. I know that go Close is not posix/linux close, but in majority of cases it'll boil down to it.
To quote:
Retrying the close() after a failure return is the wrong thing to
do, since this may cause a reused file descriptor from another
thread to be closed. This can occur because the Linux kernel
always releases the file descriptor early in the close operation,
freeing it for reuse; the steps that may return an error, such as
flushing data to the filesystem or device, occur only later in
the close operation.
Many other implementations similarly always close the file
descriptor (except in the case of EBADF, meaning that the file
descriptor was invalid) even if they subsequently report an error
on return from close(). POSIX.1 is currently silent on this
point, but there are plans to mandate this behavior in the next
major release of the standard.