#What approach should i consider when handling error which must provide some useful info?

1 messages · Page 1 of 1 (latest)

jolly fog
#

I'm making a little cli app for interpolating templates of my config files. It take paths to scan as argument and if any of them is directory i create a std.Io.Dir.Walker and iterating over them, if it's not cli just fills template and write it to a new file.

Problem is: in case of SOME errors i want to provide useful information for example if path which is provided as dest doesn't exist or app doesn't have access to it, i want to log the error and a path which caused error. If i use catch |e| and switch on error manually it creates A LOT of boilerplate code, because i want to have errors for provided config_dir, dest_paths and templating. Also because app can accept dest_path as dir OR as a file i have repeated logic like this:

# 
for (cli_args.paths) |dest_path| {
    const dest_dir_res = std.Io.Dir.openDir(.cwd(), init.io, dest_path, .{ .iterate = true });
    if (dest_dir_res) |dir| {
        defer dir.close(init.io);

        var walker = try std.Io.Dir.walk(dir, init.gpa);
        defer walker.deinit();
        while (try walker.next(init.io)) |entry| {
            if (entry.kind != .directory and std.mem.eql(
                u8,
                std.fs.path.extension(entry.basename),
                wd_ext,
            )) {
                const file = try std.Io.Dir.openFile(dir, init.io, entry.path, .{});
                logger.log("Opened dest path: {s}{c}{s}", .{ dest_path, std.fs.path.sep, entry.path });
                const buf = try readTemplateFile(init.io, init.gpa, file, colors);
                defer init.gpa.free(buf);

                const new_file_sub_path = entry.path[0 .. entry.path.len - wd_ext.len];
                try saveConfigFile(
                    init.io,
                    init.gpa,
                    .cwd(),
                    new_file_sub_path,
                    buf,
                    &cli_args,
                    &logger,
                );
            }
        }
    } else |e| {
        switch (e) {
            std.Io.Dir.OpenError.NotDir => {
                const file = try std.Io.Dir.openFile(.cwd(), init.io, dest_path, .{});
                const buf = try readTemplateFile(init.io, init.gpa, file, colors);
                defer init.gpa.free(buf);

                try saveConfigFile(
                    init.io,
                    init.gpa,
                    .cwd(),
                    dest_path[0 .. dest_path.len - wd_ext.len],
                    buf,
                    &cli_args,
                    &logger,
                );
            },
            else => return e,
        }
    }
}
#

I'm not sure how to eliminate this repeating and how i should approach errors. I was considering logging just as error appear in main, but it means that main will be responsible for logging every error in entire app. I also considered logging error in functions like fillTemplate and just return error from main, but it looks inconsistent to me, because some functions are handling logging useful info and others don't.

I tried to implement custom formatter because i found out that runtime actually logs error here:
start.zig 737-762

inline fn wrapMain(result: anytype) u8 {
    const ReturnType = @TypeOf(result);
    switch (ReturnType) {
        void => return 0,
        noreturn => unreachable,
        u8 => return result,
        else => {},
    }
    if (@typeInfo(ReturnType) != .error_union) @compileError(bad_main_ret);

    const unwrapped_result = result catch |err| {
        std.log.err("{t}", .{err});
        switch (native_os) {
            .freestanding, .other => {},
            else => if (@errorReturnTrace()) |trace| std.debug.dumpStackTrace(trace),
        }
        return 1;
    };

    return switch (@TypeOf(unwrapped_result)) {
        noreturn => unreachable,
        void => 0,
        u8 => unwrapped_result,
        else => @compileError(bad_main_ret),
    };
}

but i think that's impossible to do on error and std.log.err only uses {t} anyway

#

I also considered using struct as erros, but i will lose on ergonomics of try, ! and catch.
It does give more control but i still will have to process regular zig errors into struct and return to MY main wrapper which will print errors before exit

crystal girder
#

this is 2 questions:
for the repition, just seperate getting the file, and doing stuff with it

const file = file: {
    //...
    break :file file;
};
doStuff(file);

error reporting:
depends on what you want, but sounds like you want to report rich information to the user.
In which case I recommend making an api to collect and print the errors.
regarding inconsistent handling: if not all errors report rich information then it is not going to be the same for every error. seems like an ocd-like issue.

what that api looks like again depends, start simple and add to it as needed.

jolly fog
# crystal girder this is 2 questions: for the repition, just seperate getting the file, and doing...

I can't create a block and break out of it because in first case it's a loop with iterator of std.Io.Dir.Walker.Entry and in second case it's just an absolute path.

But i just did this

const dest_dir_res = std.Io.Dir.openDir(.cwd(), init.io, dest_path, .{ .iterate = true });
if (dest_dir_res) |dir| {
    defer dir.close(init.io);

    var walker = try std.Io.Dir.walk(dir, init.gpa);
    defer walker.deinit();
    while (try walker.next(init.io)) |entry| {
        if (entry.kind != .directory and std.mem.eql(
            u8,
            std.fs.path.extension(entry.basename),
            wd_ext,
        )) {
            try processDestFile(
                entry.dir,
                init.io,
                init.gpa,
                entry.basename,
                colors,
                cli_args.dry_run,
            );
        }
    }
} else |e| {
    switch (e) {
        std.Io.Dir.OpenError.NotDir => try processDestFile(
            .cwd(),
            init.io,
            init.gpa,
            dest_path,
            colors,
            cli_args.dry_run,
        ),
        else => return e,
    }
}

can i do something better than this?

crystal girder
#

can't create a block and break out of it because in first case it's a loop with iterator of std.Io.Dir.Walker.Entry and in second case it's just an absolute path.
?? yes you absolutely can ❗

const dir, const path = if (std.Io.Dir.openDir(.cwd(), init.io, dest_path, .{ .iterate = true })) |dir| file: {
    errdefer dir.close(init.io);
    var walker = try std.Io.Dir.walk(dir, init.gpa);
    defer walker.deinit();
    while (try walker.next(init.io)) |entry| {
        if (entry.kind != .directory and std.mem.eql(
            u8,
            std.fs.path.extension(entry.basename),
            wd_ext,
        )) {
            break :file .{ entry.dir, try init.gpa.dupe(u8, entry.basename) };
        }
    }
} else |e| switch (e) { // removed a scope level so a labled block isn't needed
      std.Io.Dir.OpenError.NotDir => .{ std.Io.Dir.cwd(), try init.gpa.dupe(dest_path) };
      else => return e,
}
defer init.gpa.free(path)
defer if (dir != .cwd()) dir.close(init.io);
try processDestFile(dir, init.io, init.gpa, path, colors, cli_args.dry_run);
#

there are multiple ways to avoid the extra allocations, they are not difficult, i was just lazy

jolly fog
crystal girder
#

no, but you can break with a tuple/array which can be destructured

#

even if you couldn't destrucure, or even make implicit tuples or tuples at all, you could still break with "multiple values" it'd just be more verbose.
even if you could only break with zero values, you could still write logic like this, you'd just have annoying vars instead of consts

#

also to be clear, there is a better way to do it, to have no extra allocations, i was just lazy

#

multiple, slightly different, ways

#

I'll leave that as an exercise for the reader zerotroll
-# definately not because I am lazy

jolly fog
#

I see a couple of ways to do less allocations. But still, won't break :file .{ entry.dir, try init.gpa.dupe(u8, entry.basename) }; just exit on first file which meets condition?

crystal girder
#

oh you wanted it to do it for each file in the dir ❗
then what you already have is fine

#

-# i cant read :3

#

i did not pay any attention to what your code was actually doing
-# a sign of a trully helpful individual zeroClueless

jolly fog
jolly fog