diff --git a/include/usage.h b/include/usage.h index fc4ff87..99c96e7 100644 --- a/include/usage.h +++ b/include/usage.h @@ -37,7 +37,11 @@ typedef struct usage_state usage_state; usage_state *usage_state_new(void); void usage_state_free(usage_state *state); -/* Advance feed state. +/* A command request is answered with the exit status and stdout. Two statuses are worth naming: + * 127 means this host will not run commands at all (a feed may then fall back to another path), + * 126 means the command could not be run (missing binary, failed spawn) and is an ordinary failure. + * + * Advance feed state. * Returns either: * {"requests":[{"kind":"http","method","url","headers":[["k","v"]],"body"?}]} * or: diff --git a/src/providers/alibaba.zig b/src/providers/alibaba.zig index c743d30..5a634d0 100644 --- a/src/providers/alibaba.zig +++ b/src/providers/alibaba.zig @@ -468,7 +468,22 @@ pub const CliReportOutcome = struct { reason: ?[]const u8 = null, }; -pub fn reportFromCli( +/// A CLI that wrote to stderr before failing has already explained itself, and that explanation is +/// more useful than the exit code: "bl is not on PATH" beats "exit 126". The body is provider-visible +/// text, so it is trimmed to one bounded line before it goes into a report. +fn cliFailureReason(alloc: std.mem.Allocator, status: i64, resp0: std.json.ObjectMap) ![]const u8 { + const body = usage.stringOf(resp0.get("body")); + if (body) |text| { + const first_line = std.mem.trim(u8, text[0..(std.mem.indexOfScalar(u8, text, '\n') orelse text.len)], " \t\r"); + if (first_line.len > 0) { + const capped = first_line[0..@min(first_line.len, 120)]; + return std.fmt.allocPrint(alloc, "bailian CLI failed (exit {d}): {s}", .{ status, capped }); + } + } + return std.fmt.allocPrint(alloc, "bailian CLI failed (exit {d})", .{status}); +} + +fn reportFromCli( alloc: std.mem.Allocator, cred_root: ?std.json.ObjectMap, resp0: std.json.ObjectMap, @@ -480,7 +495,7 @@ pub fn reportFromCli( const status = usage.toInt(i64, usage.asNumber(resp0.get("status"))) orelse -1; if (status != 0 and status != 200) { - const reason = try std.fmt.allocPrint(alloc, "bailian CLI failed (exit {d})", .{status}); + const reason = try cliFailureReason(alloc, status, resp0); return .{ .reason = reason }; } diff --git a/swift/Sources/UsageCore/UsageDriver.swift b/swift/Sources/UsageCore/UsageDriver.swift index 0290efd..cd83e75 100644 --- a/swift/Sources/UsageCore/UsageDriver.swift +++ b/swift/Sources/UsageCore/UsageDriver.swift @@ -29,9 +29,10 @@ public enum UsageDriver { do { responses.append(try await runCommand(request)) } catch { - responses.append(UsageResponse(status: 1, body: error.localizedDescription)) + responses.append(UsageResponse(status: 126, body: error.localizedDescription)) } } else { + // The one answer a feed is allowed to read as "this host will not run things". responses.append(UsageResponse(status: 127, body: "this host does not run commands (\(request.command ?? ""))")) } } else { @@ -97,8 +98,10 @@ public enum UsageDriver { process.executableURL = foundURL process.arguments = args } else { - process.executableURL = URL(fileURLWithPath: "/usr/bin/env") - process.arguments = [cmd] + args + // Not falling back to `/usr/bin/env`: it would exit 127 with the same code the protocol + // reserves for "this host does not run commands", and the feed would read a missing + // binary as a refusal. 126 says what actually happened. + return UsageResponse(status: 126, body: "\(cmd) is not on PATH") } } @@ -141,7 +144,10 @@ public enum UsageDriver { } catch { stdoutPipe.fileHandleForReading.readabilityHandler = nil stderrPipe.fileHandleForReading.readabilityHandler = nil - return UsageResponse(status: 127, body: error.localizedDescription) + // 126 is "found it, could not run it", which is not the same answer as 127 ("this host does + // not run commands"): a feed that treats 127 as "fall back to another path" must not be told + // that when the real problem is a missing binary. + return UsageResponse(status: 126, body: error.localizedDescription) } if let stdin = request.stdin, let data = stdin.data(using: .utf8) { @@ -178,9 +184,16 @@ public enum UsageDriver { let trailingErr = stderrPipe.fileHandleForReading.availableData _ = trailingErr - let body = String(data: stdoutData, encoding: .utf8) ?? "" + var body = String(data: stdoutData, encoding: .utf8) ?? "" let exitCode = Int(process.terminationStatus) + // A command that wrote nothing to stdout but failed still has something to say, and saying it + // on stderr is how a CLI reports a missing argument or a bad credential. Without this the feed + // only sees an empty body and cannot tell the user why nothing came back. + if exitCode != 0, body.isEmpty, let errorText = String(data: stderrData, encoding: .utf8), !errorText.isEmpty { + body = errorText + } + return UsageResponse(status: exitCode, body: body) }