Three hardest things in software engineering in order of difficulty:
I definitely wish cancellation was easier. For me it has been source of many bugs that are very difficult to spot, very difficult to debug, and very easy to create. And not creating them is also very unergonomic.
There is io.recancel() which makes it so next cancellation point also gets canceled. Maybe it wouldn’t be such a bad idea if that was the default behaviour. Or am I crazy?
I’m preparing to finally deploy my Zig server to production and this keeps biting me. I’m starting to regret building much of my code on top of the std.Io.Reader/Writer abstractions. The real errors are hidden behind layers of types, it’s hard to reason about the correctness of the code. I don’t see a good way forward. I find it really sad, but I’m considering going back to the generic readers/writers.
I’m building a TCP game server for the past month. Fortunately transport is a small portion of it but the issues you raise keep bugging me also. Here is my user report.
error.Canceled not being in the error set of Reader/Writer interfaces is weird to me as well. Currently I assume those operations are un-cancelable, though a different interpretation could be that their underlying Canceled operations are remapped to {Write|Read}Failed. I don’t know 
, got different problems to worry aboutcatch that would often accompany it.this is the interpretation which is in line with current reality, yes.
Adding error.Canceled to their error sets is objectively the wrong solution.
Reverting back to generic reader/writer also is not an adequate solution.
I really don’t think it’s too much to ask the programmer to unwrap ReadFailed/WriteFailed. I brush and floss my teeth every morning, no problem.
It’s tolerable when you have one layer of readers/writers, especially if you are the one creating it, and then just calling a function on the interface. But even there, it’s hard, because std.Io.File.Reader will hide the error in multiple places and you need to: 1) be aware of them 2) remember to always scan them.
It’s much harder when you deal with networking, where you are usually not the one in control of the stream, but you still need to handle these errors. I’m not saying it’s impossible to handle it correctly, but the ecosystem as a whole is broken. I can give you a number of bug reports for stdlib, where cancellation is ignored due to this.
Prime example:
https://codeberg.org/ziglang/zig/src/branch/master/lib/std/http/Client.zig#L1830
This returns error.WriteFailed and where does one look for the actual error?
There is this TODO, but it’s not really optional, this breaks cancellation.
https://codeberg.org/ziglang/zig/src/branch/master/lib/std/http/Client.zig#L1799
And even if you use the lower level APIs, there is not even getWriteError on the Connection.
There are many more cases like that in the stdlib. Every single networking package for Zig 0.15+ is broken like this.
I agree that learning to floss when you haven’t been in the habit of it can feel quite difficult! With your thoughtful insights into the problem, you seem extremely well-positioned to contribute fixes to the standard library
Thank you, this is a productive direction to take the discussion.
Is it possible to make a Reader/Writer that is generic over their vtable’s return error sets reuse the same function implementation? The machine code in each generic case would be the same (they just return the error from the vtable), so it’s just a matter of whether the compiler can merge all of these generic instances together into one usage.
If that’s possible, then you can have a reader/writer with arbitrary (caller known) errors and handle the Cancel when it’s known to be possible. As a bonus, it also allows removing the errors in cases involving infallible Readers/Writers.
I guess one take might be that if I saw similar code outside the standard library (e.g. code that “stores” the Canceled unless later queried), I’d probably see it as a code smell, depending on who’s implemented it and how it’s being used. Some of that code smell goes away when the code comes from the standard library, but we’re also strong proponents of std being a great source of idiomatic Zig for everyone to emulate.
Those two feel in contention, as this kind of storage at first glance appears as a bit of a footgun to the less experienced programmer, and one that’s hard to spot without a trained eye.
I don’t believe generic code works like this. My understanding is that a different function is instantiated each time a generic function’s comptime captures are different.
Yes, that’s my understanding as well. I’m specifically asking if it’s possible (or reasonable) for the compiler to be changed in a way to address this.
Ahhhh gotcha sorry, that reading seemed unlikely to me, since it likely represents a large amount of work.
Here is another example of the fact that it is too much to ask:
https://codeberg.org/ziglang/zig/src/branch/master/lib/std/http/Client.zig#L1846
Here, you read bodyErr() and unconditionally unwrap it. However, body_err only contains protocol-level errors, not transport-level. So if you get cancelled during discardRemaining(), body_err will be null, and your code will panic. For me, writing servers that run 24/7 unattended, where I depend on cancellation to work in order to avoid unbounded waiting, that’s unacceptable.
And the standard library is really full errors like this. That’s why I said it’s hard to reason about error handling in networking code based on std.Io.Reader/Writer. Yes, cleaning your teeth every day is one thing, this is a different level of complexity and it’s self-induced.
I see three ways out:
Accept the abstraction leak, and add error.Canceled to the error set. It’s not ideal, but error.Canceled is special enough to warrant this.
Add err: ?anyerror field in std.Io.Reader/Writer and expect the implementations to manage it when they return error.ReadFailed/WriteFailed. This seems to me like the best way out, because it gives a clean standard for developers to follow. You can either inspect it as anyerror or cast it to the concrete error type if you have access to the concrete reader/writer.
Extend the concrete readers/writers to have all the std.Io.Reader/Writer functions but doing the unwrapping. This is a messy problem to solve, and requires using anytype for readers/writers once again. Probably not acceptable for the standard library, but I’m seriously considering doing this for my projects.
This option sounds the most reasonable to me, perhaps combined with changing the contract of the vtable functions to be able to return anyerror and leave the responsibility of actually assigning to err: ?anyerror to the interface rather than implementations? This is slightly at odds with the current design, though, as there isn’t just error.ReadFailed/error.WriteFailed, but error.EndOfStream as well, and Reader’s stream may return both error.ReadFailed and error.WriteFailed depending on which end of the stream failed.
But let me brainstorm a bit: maybe this particular use case calls for something other than an error union, to be less error prone? E.g. for std.Io.Reader I could imagine the return values for stream, discard, readVec, and rebase to be as follows:
/// For `stream`
const StreamResult = union(enum) {
ok: usize,
end_of_stream,
reader_err: anyerror,
writer_err: anyerror,
};
/// For `discard` and `readVec`
const ReadResult = union(enum) {
ok: usize,
end_of_stream,
err: anyerror,
};
/// For `rebase` - this one could actually just be `anyerror!void`, but
/// for consistency, I propose this anyway.
const RebaseResult = union(enum) {
ok,
err: anyerror,
};
Yeah, that’s very non-idiomatic Zig, but it’s a practical solution to the problem. This would make it hard to forget storing the result. And the pattern when consuming error.Read/WriteFaield would be always the same, so significantly less error prone.
Thinking that code from standard library equals no code smells is wrong.
It means that it’s less likely that there are code smells, but not that there aren’t any.
I wonder why.
Yes, you would get worse/no LSP help and (likely) a lot more code generation (along worse compile times because of this), but in exchange you would solve this problem and have no vtable things need to go through.
The only other solution I could see work out is to not use the Zig error system but go with a tagged union similar to how @spiffyk proposed here.
Btw, I was randomly looking at older issues on Codeberg and found that this specific bug has been reported and has an open PR:
https://codeberg.org/ziglang/zig/issues/36296
The PR showcases the complexity, see how deep it has to reach for the error and there is still no guarantee the fix is complete.
Isn’t a canceled read or write just a “short read”? I’m not sure if it applies to zig, but in c-land you’re supposed to call fread/fwrite in a loop anyway. So I’d suppose there’s a loop around somewhere, that could check the canceled flag and break with an error.