How to do complicated cleanup

I’ve been building an IMAP client and have been running into a complicated cleanup scenario. I’m wondering if anyone has a good idea of how to handle this. The crux of it is as follows :

pub fn noop(session: *Session) !void {
     // Write out the command
     const tag: []const u8 = "N001";
     try session.writer().print("{s} NOOP\r\n", .{tag});
     try session.flush();  // Flush both buffers

     // Do some processing that will read from the buffer
     // May also allocate or parse that would result in an error
     while (true) {
         // Do processing, may fail from my bugs or unexpected input
         break;
     }

     // Cleanup. This should happen if all is well, if a recoverable error occurs, but not for an unrecoverable error
     // Need to make
     const response_tag = try session.reader().take(4);
     std.debug.assert(std.mem.eql(u8, tag, response_tag));
     // Check if OK, NO, or BAD
     _ = try session.reader().takeDelimiter('\n');
     return;

}
More Concrete Example
pub fn noop(self: *Session) !void {
    var tag_buf: [4]u8 = undefined;
    const tag = std.fmt.bufPrint(&tag_buf, "S{d:0>3}", .{self.tag_id}) catch unreachable;
    errdefer self.cleanup(tag);
    const r = self.reader();
    var w = self.writer();
    try w.print("{s} NOOP\r\n", .{tag});
    try self.flush();

    // Logic for parsing optional untagged responses
    // Would all start with `*`
   while ((try r.peekByte()) == '*') {
        // Parse it, including doing things like parsing integers or allocating memory which could fail
        _ = try r.takeDelimiter('\n');
   } 

    // Cleanup.  We need to do this if recoverable so that the read buffer doesn't contain leftovers.
    // Happy path is that the next 4 bytes would be the tag.
    // Unhappy path is we need to skip over lines until we get to the tagged line
    var line = try r.takeDelimiter('\n');
    while (!std.mem.eql(u8, line[0..4], tag)) line = try r.takeDelimiter('\n');
    // Parse the tagged value for OK, NO, or BAD
    return;
}

Here is the challenge: how do I structure this so that the cleanup only happens for recoverable errors (OutOfMemory, integer parsing, etc.) but not for std.Io.Reader.Errors that indicate the stream is closed or some unrecoverable error.
Ideally this is repeatable for the other commands that IMAP supports, not just the NOOP command.

errdefer can capture the error, and then you can operate on it.

errdefer |err| switch (err) {
    error.OutOfMemory, ... => {
        // Handle cleanup
    },
    else => {}, // bail out
}
1 Like
Is it possible that the same error is recoverable in one part of code but not in another?  

You need goto, which in Zig is disguised as named switch prongs:

pub fn noop(self: *Session) !void {
    var tag_buf: [4]u8 = undefined;
    const tag = std.fmt.bufPrint(&tag_buf, "S{d:0>3}", .{self.tag_id}) catch unreachable;
    errdefer self.cleanup(tag);
    const r = self.reader();
    var w = self.writer();
    try w.print("{s} NOOP\r\n", .{tag});
    try self.flush();

    var return_err: anyerror = undefined;
  
    const Prongs = enum{ ok, err };
    sw: switch(Prongs.ok){
      .ok => while ((try r.peekByte()) == '*') {
        _ = try r.takeDelimiter('\n');
        if(recoverable_error) |e| {
          return_err = e;
          continue :sw .err;
        }
      },
        .err =>  {
          var line = try r.takeDelimiter('\n');
          while (!std.mem.eql(u8, line[0..4], tag)) line = try r.takeDelimiter('\n');
          return return_err;
    }
   
}
2 Likes

Right, but that doesn’t handle the happy path. If there are no recoverable errors that happen, the cleanup also needs to run. As far as I know, defer does not capture so I can’t just defer here

Note that removing the support for captures in errdefer is an accepted proposal

4 Likes

That’s a good question. I don’t have the answer right now, but that may point to the fact that I need to review the error sets and determine more concretely if there is overlap.

At this point, I don’t think they overlap. So any error other than those in std.Io.Reader/std.Io.Writer would be “recoverable”. All others would need cleanup.

It’s kind of convoluted and definitely not super idiomatic, but I came up with this solution.

EDIT: You can zig run this. Change .step to different values, like 9 and 3, to get different results:

const MyError = error{ Recoverable, Fatal };

pub fn main() !void {
    var state = State{ .step = 5 };

    outer: switch(@as(union(enum) {
        cleanup,
        err: MyError,
    }, inner: {
        // Do the stuff that may error, but instead of tries, break the inner
        // block in the catch.
        while (state.fallibleOp() catch |e| break :inner .{ .err = e }) {
            _ = state.fallibleOp() catch |e| break :inner .{ .err = e };
            _ = state.fallibleOp() catch |e| break :inner .{ .err = e };
        }

        // If everything is okay, go to normal cleanup
        break :inner .cleanup;
    })) {
        .cleanup => {
            std.log.err("cleanup", .{});
        },
        .err => |e| switch (e) {
            error.Recoverable => {
                // Maybe do some recovery operations here? Then return to normal
                // cleanup.
                continue :outer .cleanup;
            },
            error.Fatal => {
                // Do not do normal clean up.
                std.log.err("FATAL", .{});
            },
        },
    }
}


// Unimportant implementation details below:

const std = @import("std");
const State = struct {
    step: u32,

    fn fallibleOp(self: *State) MyError!bool {
        if (self.step == 0)
            return false;

        if (self.step % 4 == 0)
            return error.Recoverable;
        if (self.step % 9 == 0)
            return error.Fatal;
        self.step -= 1;
        return true;
    }
};
1 Like
fn partDefer(
    condition1: bool,
    condition2: bool,
    conditoin3: bool,
    resource: *bool,
) !usize {
    const Recoverable = enum {
        no_error,
        recoverable_error,
        non_recoverable_error,
        fn check(self: *@This(), result: anytype) @TypeOf(result) {
            _ = result catch |err| {
                if (err == error.MyRecoverable or
                    err == error.MyRecoverable2)
                    self.* = .recoverable_error
                else
                    self.* = .recoverable_error;
            };
            return result;
        }
    };
    var recoverable: Recoverable = .no_error;
    defer switch (recoverable) {
        .no_error, .recoverable_error => resource.* = false,
        else => {},
    };
    try recoverable.check(testFn1(condition1));
    const result = try recoverable.check(testFn2(condition2));
    try recoverable.check(testFn3(conditoin3));
    return result;
}
fn testFn1(condition: bool) !void {
    if (condition) return else return error.MyRecoverable;
}
fn testFn2(condition: bool) !usize {
    if (condition) return 1 else return error.MyRecoverable2;
}
fn testFn3(condition: bool) !void {
    if (condition) return else return error.MyUnRecoverable;
}
test partDefer {
    var resource = true;
    var result = partDefer(false, true, true, &resource);
    try std.testing.expect(result == error.MyRecoverable and resource == false);
    resource = true;
    result = partDefer(true, false, true, &resource);
    try std.testing.expect(result == error.MyRecoverable2 and resource == false);
    resource = true;
    result = partDefer(true, true, false, &resource);
    try std.testing.expect(result == error.MyUnRecoverable and resource == true);
    const res = try partDefer(true, true, true, &resource);
    try std.testing.expect(res == 1 and resource == false);
}

The above code has been tested. I attempted to rewrite the code of OP using this solution, but I haven’t tested this version.

One drawback of this approach is that you have to remember to use recoverable.check at all error call points. But this can be compensated for by the assertion of errdefer, which triggers the assertion once the error returns without going through recoverable.check.

pub fn noop(self: *Session) !void {
    var recoverable: enum {
        no_error,
        recoverable_error,
        non_recoverable_error,
        fn check(self: *@This(), result: anytype) @TypeOf(result) {
            _ = result catch |err| {
                if (err == error.ReadFailed or
                    err == error.WriteFailed)
                    self.* = .non_recoverable_error
                else
                    self.* = .recoverable_error;
            };
            return result;
        }
    } = .no_error;
    var tag_buf: [4]u8 = undefined;
    const tag = std.fmt.bufPrint(&tag_buf, "S{d:0>3}", .{self.tag_id}) catch unreachable;
    const r = self.reader();
    var w = self.writer();
    try recoverable.check(w.print("{s} NOOP\r\n", .{tag}));
    try recoverable.check(self.flush());
    defer switch (recoverable) {
        .no_error, .recoverable_error => {
            // Cleanup.  We need to do this if recoverable so that the read buffer doesn't contain leftovers.
            // Happy path is that the next 4 bytes would be the tag.
            // Unhappy path is we need to skip over lines until we get to the tagged line
            var line = r.takeDelimiter('\n') catch unreachable;
            while (!std.mem.eql(u8, line[0..4], tag)) line = r.takeDelimiter('\n') catch unreachable;
            // Parse the tagged value for OK, NO, or BAD
        },
        else => {},
    };
    errdefer if (recoverable == .no_error) @panic("miss recoverable check!");

    // Logic for parsing optional untagged responses
    // Would all start with `*`
   while ((try recoverable.check(r.peekByte())) == '*') {
        // Parse it, including doing things like parsing integers or allocating memory which could fail
        _ = try recoverable.check(r.takeDelimiter('\n'));
   } 
}