Rich error reporting

I want to develop a rich error reporting mechanism for zig. I understands that this involves some overhead.

My personal c code uses this struct, which isn’t very rich, but its quite helpful.

typedef struct {
  int code; // non-zero means an error happened
  const char *msg;
  const char *func;
} Ret;

#define RET ((Ret){0, SUCCESS_STR, __func__})

#define FN_RET(code, msg) ((Ret){code, msg, __func__})

#define FN_ERR() ((Ret){errno, strerror(errno), __func__})

This is generally fine, and i think the overhead pays for itself in the time it has saved me. One problem with this approach is that it is limited on what information can be shown in the msg field.
Sometimes when an error happens in a nested function, only one Ret struct can be propagated up, this means that I have to choose which level of detail to retain. This sucks because many times the error happens because of combined function contexts. I can live with this since I only use it in my personal code and I tend to know my code base fairly well.

However, at work we use cpp, and many times errors have to be passed between different contexts (file streams, Graphical interfaces, stdout, threads, etc..). For this reason i decided to do something like this.

class Ret{
public:
  int code;
  std::string message;
};

I understand the cost of passing around a dynamic string, but again this is not a logging mechanism, it is an error reporting one. Only error information is processed through this object. The context is appended to the string, we like that we can include formatted strings in the message and be explicit about the error.

I attempted something to that effect in zig, but i think i failed miserably.

This is the function that builds the type

fn ReturnValue(T: type, E: type) type {
    const tag = enum { ok, fault };
    return union(tag) {
        const Self = @This();
        ok: T,
        fault: struct {
            reason: E,
            message: []const u8,
            maybe_allocator: ?std.mem.Allocator,

            pub fn deinit(self: @This()) void {
                if (self.maybe_allocator) |allocator| {
                    allocator.free(self.message);
                }
            }
        },

        fn succeed(value: T) Self {
            return .{ .ok = value };
        }

        fn fail(pReason: E, pMessage: []const u8, pMaybe_allocator: ?std.mem.Allocator) Self {
            return .{
                .fault = .{
                    .reason = pReason,
                    .message = pMessage,
                    .maybe_allocator = pMaybe_allocator,
                },
            };
        }
    };
}

This is how an object incorporates it

const Object = struct {
    const Self = @This();
    const Error = error{ InvalidArgument, OutOfBounds, NoEnoughArguments };
    const Ret = ReturnValue(Self, Error);
    allocator: std.mem.Allocator,

    // Example usage: a more practical case would have more complicated errors.
    pub fn init(args_vector: []const [*:0]const u8, pAllocator: std.mem.Allocator) !Ret {
        if (args_vector.len < 2) {
            return .fail(Error.NoEnoughArguments, "Not enough arguments", null);
        }

        if (args_vector.len == 2) {
            return .fail(
                Error.InvalidArgument,
                try std.fmt.allocPrint(pAllocator, "Invalid parameter: {s}\n", .{args_vector.ptr[1]}),
                pAllocator,
            );
        }

        return .succeed(.{ .allocator = pAllocator });
    }
};

The thing that makes this feel like a failure is the overhead associated with using it, and also, propagating errors does not seem like it would be super streamlined.

    const object = switch (try Object.init(init.minimal.args.vector, gpa.allocator())) {
        .ok => |value| value,
        .fault => |e| {
            std.log.err("Error: {s}\n", .{e.message});
            e.deinit();
            return e.reason;
        },
    };

    _ = object;

Does anyone have an idea on how to improve this rich error reporting mechanism? Or a better way of doing it?

1 Like

A common way to do something like this is to have an optional parameter for diagnostics to be passed in. If you search for “diagnostics pattern” in this forum or in your favorite search engine you should find some threads/blogs about it. You can also see how it’s defined and used in something like std.json and std.zon.

Another thing that might be useful is to not allocate the message but to just print it into a buffer.

So in total something like this:

const std = @import("std");

const Diagnostic = struct {
    err: error{NegativeInt}, // TODO: change with your error set
    message: []const u8,
    buf: [128]u8,
};

fn yourFunction(a: f32, diag: ?*Diagnostic) error{NegativeInt}!f32 {
    if (a >= 0) return @sqrt(a);

    if (diag) |d| {
        d.message = std.fmt.bufPrint(
            &d.buf,
            "Expected nonnegative number. Got: {}",
            .{a},
        ) catch unreachable;
    }
    return error.NegativeInt;
}

pub fn main() !void {
    var diag: Diagnostic = undefined;
    const b = yourFunction(-1, &diag) catch |err| {
        std.log.err("Got: {}, diag: {s}", .{ err, diag.message });
        return err;
    };
    std.log.info("Got: {}", .{b});
}
1 Like

Expounding on this:

Storing the error in the struct is redundant if also returning it, unless it stores a more specific error.

Storing just a string makes it limited to very basic reporting to user, if you want to do control flow based on the contents, or more sophisticated and consistent error reporting, or both, you are better off storing much more tailored data.

For example, std.json.Diagnostic only stores where in parsing it was, the specific error is returned.

Lastly, the message slice being self-referential to an array is error-prone, and redundant; just have a len field instead, and functions to get and print to it.

5 Likes

Thanks for the info.

Passing the ReturnValue/Context was something I considered briefly, it seemed ideal to use the language payload capture or case matching ergonomics directly with the function, otherwise it seems more like a work-around.

I did not know about diagnostics patterns, i will definitely take a look.

1 Like

I’ve seen an interesting take on this using tagged unions, in the ‘argzon’ library while I was updating it to zig-0.16.0.

Basically, the function returns a tagged union of the actual value type and a diagnostic type:

https://codeberg.org/vincent-dalstra/argzon/src/commit/672a7a19c98adb4fd08463ab9feadf113bfa16e4/src/args.zig#L292-L295

return struct {
    const Self = @This();
     ...
    
    const ParseCommandResult = union(enum) {
        self: Self,
        diag: Diagnostic,
    };
    ...

    /// Parse command arguments.
    fn parseCommand(
        allocator: std.mem.Allocator,
        arg_str_iter: anytype,
        writer: *std.Io.Writer,
        opts: ParseOptions,
    ) Error!ParseCommandResult
}

In this case, ‘Diagnostic’ is itself a tagged union:

https://codeberg.org/vincent-dalstra/argzon/src/commit/672a7a19c98adb4fd08463ab9feadf113bfa16e4/src/args.zig#L12-L21

/// Command-line argument parsing diagnostic.
const Diagnostic = union(enum) {
    Help,
    NoArgumentValue: [:0]const u8,
    UnexpectedArgument: [:0]const u8,
    UnexpectedSubcommand: [:0]const u8,
    UnexpectedArgumentValue: [2][:0]const u8,
    PresentNamedArgumentExclude: [3][:0]const u8,
    MissingNamedArgumentDependencies: [:0]const u8,
};

Then you switch on the return value to determine if it’s the ‘real’ return value or a diagnostic. If it is a diagostic, switch again to print it correctly.

https://codeberg.org/vincent-dalstra/argzon/src/commit/672a7a19c98adb4fd08463ab9feadf113bfa16e4/src/args.zig#L310-L336

...
// Parse command-line arguments, exit process successfully on help.
return switch (try parseCommand(allocator, &arg_str_iter, writer, opts)) {
    .self => |self| self,
    .diag => |diag| switch (diag) {
        inline else => |payload, tag| {
            if (tag != .Help) try writer.print("error.{t}: ", .{tag});
            switch (tag) {
                .Help => {
                    try writeHelp(writer, .{});
                    try writer.flush();
                    std.process.exit(0);
                },
                .NoArgumentValue,
                .UnexpectedArgument,
                => try writer.writeAll(payload),
                .UnexpectedSubcommand => try writer.writeAll(payload),
                .UnexpectedArgumentValue => try writer.print("--{s} cannot be equal to {s}", .{ payload[0], payload[1] }),
                .PresentNamedArgumentExclude => try writer.print("--{s} excludes --{s} {s}", .{ payload[0], payload[1], payload[2] }),
                .MissingNamedArgumentDependencies => try writer.print("--{s}", .{payload}),
            }
            try writer.writeAll("\n\n");
            try writeUsage(writer);
            try writer.flush();
            std.process.exit(2);
        },
    },
};
1 Like

I like the idea incorporating the writer in the mechanism, its something I thought about but haven’t done. that might be a good approach although having to pass a writer plus an allocator does seem to add more work than my current approach.

Error reporting, similar to CLI argument parsing, is a surprisingly vast topic. There are too many strategies to count.

The diagnostics pattern mentioned above is popular in Zig, and is probably one of the most common techniques for rendering errors in a more robust way other than just printing the error (e.g. error.SomethingWentWrong).

In your example usage of a Result-ish type, you print the error message and return the error code immediately. Taking this at face value, wouldn’t it be simpler to just print the error message in the called function and call it a day? If you truly do need to save the result of this call for later use, obviously this won’t work, but if not I would consider reducing the complexity here.

Another option I’ve been trying out in my own projects is the Diagnostics Factory. This allows more robust error reporting and naturally supports deferring the reporting of the errors. Take a look at the example in that blog post, and let me know if you’d like to see how I’m using this pattern in my own code.

In my opinion, the diagnostics pattern (not the diagnostics factory) is noisy because you have to change the signature of your API to accept the diagnostics argument, which is not optional. You either have to pass it by reference, or pass null. In contrast, the diagnostics factory is a higher order concept and separates both the collection and reporting from your API, leaving your API unchanged.

4 Likes

Thanks for the input, I didn’t realize that I would not be able to append to the slice created, I had to change my code to accept a type that i could resize/append to, also the fault struct was changed to support the append method; i left the changes out to avoid clutter. I also realized that am not able to edit the original post to reflect that i changed the name from ReturnValue to Result. Sorry about that guys.

As far as your suggestion, I think its a good one and perhaps I would need to implement that in a larger project before I could give any meaningful feedback. For now I am mostly interested in something that integrates with the language ergonomics.

My changes based on your observation are as follows
The message field is now a struct that looks like this.

const Message = struct {
    context_is_complete: bool = true,
    text: std.ArrayList(u8) = .empty,
    maybe_allocator: ?std.mem.Allocator = std.heap.page_allocator,

    fn init(comptime fmt: []const u8, args: anytype, allocator: std.mem.Allocator) @This() {
        const string = std.fmt.allocPrint(allocator, fmt, args) catch
            return .{
                .context_is_complete = false,
                .maybe_allocator = null,
            };
        defer allocator.free(string);
        var message: @This() = .{ .maybe_allocator = allocator };

        message.text.appendSlice(allocator, string) catch return .{
            .context_is_complete = false,
            .maybe_allocator = null,
        };

        return message;
    }

    fn deinit(self: *@This()) void {
        if (self.maybe_allocator) |allocator| {
            self.text.deinit(allocator);
        }
    }
};

It allows me to propagate the error up as shown below. One thing to note is that the result has to be deinit’d once it will not be propagated up.

fn someFunction(init: std.process.Init, allocator: std.mem.Allocator) Result(Object, Object.Error) {
    var result: Result(Object, Object.Error) = Object.init(init.minimal.args.vector, allocator);
    const object = switch (result) {
        .ok => |value| value,
        .fault => |*e| {
            e.append(" This is just extra {s}.\n", .{"stuff"});
            return result;
        },
    };
    return .succeed(object);
}

Then the function above would be

    var result = someFunction(init, gba.allocator());
    const object = switch (result) {
        .ok => |value| value,
        .fault => |*e| {
            std.log.err("Error: {s}\n", .{e.context()});
            defer e.deinit();
            return e.reason;
        },
    };
    _ = object;

and the stdout

> zig build-exe main.zig && ./main
error: Error: Not enough arguments This is just extra stuff.
1 Like