Skip to content

Preserve error context across command boundaries #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jhuntwork merged 1 commit into from
Aug 23, 2026
Merged

Preserve error context across command boundaries #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/activation.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const store = @import("store.zig");

/// Activation error set
const Std = errors.StandardErrors;
pub const ActivationError = Std.OutOfMemory || Std.FileSystem || Std.PermissionDenied || Std.InvalidInput || error{
pub const ActivationError = Std.OutOfMemory || Std.FileSystem || Std.PermissionDenied || Std.InvalidInput || Std.CorruptData || error{
GenerationNotFound, // Target generation doesn't exist
ManifestNotFound, // Generation exists but manifest is missing/unreadable
DuplicateEtcTemplate, // Two packages provide same /etc path
Expand Down Expand Up @@ -71,7 +71,7 @@ pub fn switchProfileGeneration(
};
defer ctx.allocator.free(store_root_a);
gcroots.updateRoots(ctx.allocator, store_root_a, gc_roots_dir, profile_dir, gcroots.DEFAULT_RETENTION_COUNT) catch |err| {
return mapGCRootError(err);
return ctx.fail(mapGCRootError(err), gc_roots_dir, "failed to update GC roots after activation");
};
}

Expand Down Expand Up @@ -110,7 +110,7 @@ pub fn activateSystemGeneration(
};
defer ctx.allocator.free(store_root_b);
gcroots.updateRoots(ctx.allocator, store_root_b, gc_roots_dir, profile_dir, gcroots.DEFAULT_RETENTION_COUNT) catch |err| {
return mapGCRootError(err);
return ctx.fail(mapGCRootError(err), gc_roots_dir, "failed to update GC roots after activation");
};

const result = SystemActivationResult{
Expand Down Expand Up @@ -249,10 +249,10 @@ fn loadValidatedTargetManifest(
};
return ctx.fail(switch (err) {
generation.GenerationError.OutOfMemory => ActivationError.OutOfMemory,
generation.GenerationError.GenerationNotFound,
generation.GenerationError.InvalidManifest,
generation.GenerationError.ParseError,
=> ActivationError.ManifestNotFound,
=> ActivationError.CorruptData,
generation.GenerationError.GenerationNotFound => ActivationError.ManifestNotFound,
else => ActivationError.FileSystem,
}, manifest_path, detail);
};
Expand Down Expand Up @@ -522,7 +522,7 @@ fn validateGenerationStorePaths(
defer ctx.allocator.free(normalized_store_path);

if (!path_safety.isWithinBoundary(normalized_store_path, normalized_store_root)) {
return ctx.fail(ActivationError.InvalidInput, pkg.store_path, "store path outside store root");
return ctx.fail(ActivationError.CorruptData, pkg.store_path, "store path outside store root");
}

var store_dir = path_mod.openExistingDir(pkg.store_path) catch |err| {
Expand Down Expand Up @@ -550,14 +550,14 @@ fn validateGenerationStorePaths(

if (do_hash_verify) {
if (pkg.content_hash.len != 64) {
return ctx.fail(ActivationError.InvalidInput, pkg.store_path, "invalid content hash length in manifest");
return ctx.fail(ActivationError.CorruptData, pkg.store_path, "invalid content hash length in manifest");
}

const format = package_manifest.detectFormat(ctx.allocator, pkg.store_path) catch |err| {
return ctx.fail(switch (err) {
package_manifest.ManifestError.OutOfMemory => ActivationError.OutOfMemory,
package_manifest.ManifestError.PermissionDenied => ActivationError.PermissionDenied,
package_manifest.ManifestError.InvalidInput => ActivationError.InvalidInput,
package_manifest.ManifestError.InvalidInput => ActivationError.CorruptData,
else => ActivationError.FileSystem,
}, pkg.store_path, "failed to detect package manifest format");
};
Expand All @@ -571,7 +571,7 @@ fn validateGenerationStorePaths(
return ctx.fail(switch (err) {
hash.HashError.OutOfMemory => ActivationError.OutOfMemory,
hash.HashError.PermissionDenied => ActivationError.PermissionDenied,
hash.HashError.InvalidInput => ActivationError.InvalidInput,
hash.HashError.InvalidInput => ActivationError.CorruptData,
else => ActivationError.FileSystem,
}, pkg.store_path, "failed to compute store content hash");
};
Expand All @@ -587,7 +587,7 @@ fn validateGenerationStorePaths(
}
}
if (!accepted_transitional) {
return ctx.fail(ActivationError.InvalidInput, pkg.store_path, "store content hash mismatch");
return ctx.fail(ActivationError.CorruptData, pkg.store_path, "store content hash mismatch");
}
}
}
Expand Down Expand Up @@ -1154,7 +1154,7 @@ test "system profile activation rejects mismatched content hash without verify-s
// Activate with .fast (no explicit verify-store) — should STILL fail
// because system profile hash verification is now mandatory
try std.testing.expectError(
ActivationError.InvalidInput,
ActivationError.CorruptData,
activateSystemGeneration(&test_env.ctx, 2, .fast),
);

Expand Down
33 changes: 23 additions & 10 deletions src/cli/cli.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,19 @@ pub const CLI = struct {
var parse_args_with_program = std.ArrayList([]const u8).empty;
defer parse_args_with_program.deinit(self.allocator);
parse_args_with_program.append(self.allocator, args[0]) catch {
return 1;
emitFormattedCliError(ctx, null, MereError.OutOfMemory, null);
return command.exitCodeForError(MereError.OutOfMemory);
};
for (prescan_result.filtered_args) |arg| {
parse_args_with_program.append(self.allocator, arg) catch {
return 1;
emitFormattedCliError(ctx, null, MereError.OutOfMemory, null);
return command.exitCodeForError(MereError.OutOfMemory);
};
}

var inferred_command_path = self.inferCommandPath(parse_args_with_program.items) catch {
return 1;
var inferred_command_path = self.inferCommandPath(parse_args_with_program.items) catch |err| {
emitFormattedCliError(ctx, null, err, null);
return command.exitCodeForError(err);
};
defer inferred_command_path.deinit(self.allocator);

Expand All @@ -100,10 +103,16 @@ pub const CLI = struct {

// Store verbose flag in parsed_args for consistency
if (prescan_result.verbose) {
parsed_args.global_flags.put("verbose", types.FlagValue{ .bool = true }) catch {};
parsed_args.global_flags.put("verbose", types.FlagValue{ .bool = true }) catch {
emitFormattedCliError(ctx, commandPhase(parsed_args.command_path), MereError.OutOfMemory, null);
return command.exitCodeForError(MereError.OutOfMemory);
};
}
if (prescan_result.no_color) {
parsed_args.global_flags.put("no-color", types.FlagValue{ .bool = true }) catch {};
parsed_args.global_flags.put("no-color", types.FlagValue{ .bool = true }) catch {
emitFormattedCliError(ctx, commandPhase(parsed_args.command_path), MereError.OutOfMemory, null);
return command.exitCodeForError(MereError.OutOfMemory);
};
}

// Handle case where no command was specified - this is a usage error
Expand Down Expand Up @@ -323,10 +332,14 @@ pub const CLI = struct {
return 2; // Usage error for invalid flags
};

// Execute the command
const result = cmd.handler(ctx, args) catch |err| {
emitFormattedCliError(ctx, commandPhase(args.command_path), err, null);
return 1; // Execution error
// Execute the command. This is the final error boundary: handlers may
// return a CommandResult themselves, but any propagated MereError still
// receives the same diagnostic formatting and exit-code mapping.
const result = cmd.handler(ctx, args) catch |err| blk: {
break :blk command.errorResult(ctx, err, null) catch {
emitFormattedCliError(ctx, commandPhase(args.command_path), err, null);
return command.exitCodeForError(err);
};
};

if (result.segments) |segments| {
Expand Down
29 changes: 16 additions & 13 deletions src/cli/command.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,8 @@ pub fn exitCodeForError(err: MereError) u8 {
/// a CommandResult the handler should return immediately.
pub fn acquireStoreLockOrResult(ctx: *mere.Context) !?types.CommandResult {
ctx.acquireStoreLock() catch |err| {
return types.CommandResult{
.success = false,
.exit_code = exitCodeForError(mere.errors.ErrorMapping.mapZigError(err)),
.message = try ctx.allocator.dupe(u8, mere.errors.getUserFriendlyMessage(err)),
};
ctx.setDiagnosticContext(ctx.root_path, "failed to acquire store lock");
return try errorResult(ctx, err, null);
};
return null;
}
Expand Down Expand Up @@ -291,18 +288,24 @@ pub fn errorResult(ctx: *mere.Context, err: anyerror, message_override: ?[]const
};
}

test "errorResult maps the exit code correctly instead of hardcoding it" {
test "errorResult preserves the standard vocabulary's exit-code classes" {
const testing = std.testing;
var ctx = mere.Context.init(testing.allocator, "/test");
defer ctx.deinit();

// Regression: every CLI handler's error boundary used to hardcode
// exit_code = 1 regardless of the underlying error, so a
// PermissionDenied surfaced identically to an out-of-memory failure.
const result = try errorResult(&ctx, error.PermissionDenied, null);
defer ctx.allocator.free(result.message.?);
try testing.expectEqual(@as(u8, 13), result.exit_code);
try testing.expect(!result.success);
const cases = [_]struct { err: anyerror, exit_code: u8 }{
.{ .err = error.InvalidInput, .exit_code = 2 },
.{ .err = error.PermissionDenied, .exit_code = 13 },
.{ .err = error.OutOfMemory, .exit_code = 12 },
.{ .err = error.OutOfDisk, .exit_code = 12 },
.{ .err = error.TooManyFiles, .exit_code = 12 },
};
for (cases) |case| {
const result = try errorResult(&ctx, case.err, null);
defer ctx.allocator.free(result.message.?);
try testing.expectEqual(case.exit_code, result.exit_code);
try testing.expect(!result.success);
}
}

test "errorResult folds existing diagnostic context into the message" {
Expand Down
93 changes: 15 additions & 78 deletions src/cli/commands/build.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ const download = mere.download;
const build = mere.build;
const DiagnosticContext = mere.errors.DiagnosticContext;
const getUserFriendlyMessage = mere.errors.getUserFriendlyMessage;
const ui = mere.ui;
const emit = ui.emit;

/// Build (dev) subcommand metadata
const build_meta = command.CommandMeta{
Expand Down Expand Up @@ -51,116 +49,53 @@ fn handleBuild(ctx: *mere.Context, args: *const types.ParsedArgs) MereError!type

// Ensure configuration is loaded for dependency resolution
_ = ctx.getConfig() catch |err| {
// Enrich diagnostic context for configuration load failures
ctx.setDiagnosticContext("configuration", "failed to load configuration");
const user_message = getUserFriendlyMessage(err);
const error_ctx = ctx.getDiagnosticContext().toErrorContext();
const formatted_message = error_ctx.formatWithMessage(ctx.allocator, user_message) catch user_message;

return types.CommandResult{
.success = false,
.exit_code = 1,
.message = try std.fmt.allocPrint(ctx.allocator, "Configuration error: {s}", .{formatted_message}),
};
return try command.errorResult(ctx, err, "configuration error");
};

// Initialize real curl-backed transfer client for the build request.
var curl_client = download.CurlTransferClient.init(ctx, command.user_agent) catch |err| {
// Add diagnostic context for download initialization failures
ctx.setDiagnosticContext(recipe_path, "failed to initialize download client");
// Get user-friendly error message and format with context
const user_message = getUserFriendlyMessage(err);
const error_ctx = ctx.getDiagnosticContext().toErrorContext();
const formatted_message = error_ctx.formatWithMessage(ctx.allocator, user_message) catch user_message;

return types.CommandResult{
.success = false,
.exit_code = 1,
.message = try ctx.allocator.dupe(u8, formatted_message),
};
return try command.errorResult(ctx, err, null);
};
defer download.CurlTransferClient.cleanupFn(ctx, curl_client);
const client = curl_client.client();

// Load recipe file contents into memory (allocator-owned buffer)
var buf_path: [std.fs.max_path_bytes]u8 = undefined;
const abs_recipe_path = path.resolveToAbsolutePath(recipe_path, &buf_path) catch |err| {
// Enrich diagnostic context for path resolution failures
ctx.setDiagnosticContext(recipe_path, "failed to resolve recipe path");
// Get user-friendly error message
const user_message = getUserFriendlyMessage(err);

return types.CommandResult{
.success = false,
.exit_code = 1,
.message = try std.fmt.allocPrint(ctx.allocator, "Unable to resolve recipe path '{s}': {s}", .{ recipe_path, user_message }),
};
return try command.errorResult(ctx, err, null);
};

var recipe_file = path.openExistingFile(abs_recipe_path) catch |err| {
// Enrich diagnostic context for file open failures
ctx.setDiagnosticContext(abs_recipe_path, "failed to open recipe file");
// Get user-friendly error message
const user_message = getUserFriendlyMessage(err);

return types.CommandResult{
.success = false,
.exit_code = 1,
.message = try std.fmt.allocPrint(ctx.allocator, "Unable to open recipe '{s}': {s}", .{ abs_recipe_path, user_message }),
};
return try command.errorResult(ctx, err, null);
};
defer recipe_file.close(path.currentIo());

// Prefer explicit size read to avoid readToEndAlloc FileTooBig errors and to validate size.
const file_size = (recipe_file.stat(path.currentIo()) catch |err| {
// Enrich diagnostic context for stat failures
ctx.setDiagnosticContext(abs_recipe_path, "failed to stat recipe file");
// Get user-friendly error message
const user_message = getUserFriendlyMessage(err);

return types.CommandResult{
.success = false,
.exit_code = 1,
.message = try std.fmt.allocPrint(ctx.allocator, "Unable to stat recipe '{s}': {s}", .{ abs_recipe_path, user_message }),
};
return try command.errorResult(ctx, err, null);
}).size;

if (file_size > 1024 * 1024 * 10) {
// Enrich diagnostic context for oversized recipe files
ctx.setDiagnosticContext(abs_recipe_path, "recipe file too large");

return types.CommandResult{
.success = false,
.exit_code = 1,
.message = try std.fmt.allocPrint(ctx.allocator, "Recipe file too large: {s}", .{abs_recipe_path}),
};
return try command.errorResult(ctx, MereError.InvalidInput, "recipe file too large");
}

const recipe_buf = try ctx.allocator.alloc(u8, file_size);
defer ctx.allocator.free(recipe_buf);

const bytes_read = recipe_file.readPositionalAll(path.currentIo(), recipe_buf, 0) catch |err| {
// Enrich diagnostic context for read failures
ctx.setDiagnosticContext(abs_recipe_path, "failed to read recipe file");
// Get user-friendly error message
const user_message = getUserFriendlyMessage(err);

return types.CommandResult{
.success = false,
.exit_code = 1,
.message = try std.fmt.allocPrint(ctx.allocator, "Unable to read recipe '{s}': {s}", .{ abs_recipe_path, user_message }),
};
return try command.errorResult(ctx, err, null);
};

if (bytes_read != file_size) {
// Enrich diagnostic context for short reads
ctx.setDiagnosticContext(abs_recipe_path, "short read while reading recipe file");

return types.CommandResult{
.success = false,
.exit_code = 1,
.message = try std.fmt.allocPrint(ctx.allocator, "Short read for recipe '{s}'", .{abs_recipe_path}),
};
return try command.errorResult(ctx, MereError.FileSystem, "short read while reading recipe file");
}

var request = build.BuildRequest.init();
Expand Down Expand Up @@ -212,11 +147,13 @@ fn handleBuild(ctx: *mere.Context, args: *const types.ParsedArgs) MereError!type
}
}

emit.diagnostic(ctx, .build, "build failed", diagnostic_subject, diagnostic_details, base_message);
return types.CommandResult{
.success = false,
.exit_code = 1,
};
if (diagnostic_subject != null or diagnostic_details != null) {
ctx.withDiagnosticContext(DiagnosticContext{
.subject = diagnostic_subject,
.details = diagnostic_details,
});
}
return try command.errorResult(ctx, err, base_message);
};
defer result.deinit();

Expand Down
Loading