Skip to content

Commit cbbabf0

Browse files
chaliyclaude
chaliy
and
claude
authored
fix(jq): return ExecResult::err instead of Error::Execution for stderr suppression (#298)
## Summary - jq builtin returned `Err(Error::Execution(...))` for parse/runtime errors - This bypassed `apply_redirections`, so `2>/dev/null` couldn't suppress errors - Changed all error paths to return `Ok(ExecResult::err(...))` with non-zero exit code - Errors now flow through normal redirect handling ## Test plan - [x] `issue_277_jq_stderr_suppressed` — `jq '.foo' 2>/dev/null` suppresses error - [x] All 41 existing jq unit tests pass (updated 5 that expected `is_err()`) Closes #277 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3857cb7 commit cbbabf0

File tree

2 files changed

+164
-58
lines changed

2 files changed

+164
-58
lines changed

‎crates/bashkit/src/builtins/jq.rs‎

Lines changed: 141 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,15 @@ impl Builtin for Jq {
213213
// --argjson name value: bind $name to parsed JSON value
214214
if i + 2 < ctx.args.len() {
215215
let name = format!("${}", &ctx.args[i + 1]);
216-
let json_val: serde_json::Value = serde_json::from_str(&ctx.args[i + 2])
217-
.map_err(|e| {
218-
Error::Execution(format!("jq: invalid JSON for --argjson: {}", e))
219-
})?;
216+
let json_val: serde_json::Value = match serde_json::from_str(&ctx.args[i + 2]) {
217+
Ok(v) => v,
218+
Err(e) => {
219+
return Ok(ExecResult::err(
220+
format!("jq: invalid JSON for --argjson: {}\n", e),
221+
2,
222+
));
223+
}
224+
};
220225
var_bindings.push((name, json_val));
221226
i += 3;
222227
continue;
@@ -298,15 +303,19 @@ impl Builtin for Jq {
298303
path: (),
299304
};
300305

301-
let modules = loader.load(&arena, program).map_err(|errs| {
302-
Error::Execution(format!(
303-
"jq: parse error: {}",
304-
errs.into_iter()
305-
.map(|e| format!("{:?}", e))
306-
.collect::<Vec<_>>()
307-
.join(", ")
308-
))
309-
})?;
306+
let modules = match loader.load(&arena, program) {
307+
Ok(m) => m,
308+
Err(errs) => {
309+
let msg = format!(
310+
"jq: parse error: {}\n",
311+
errs.into_iter()
312+
.map(|e| format!("{:?}", e))
313+
.collect::<Vec<_>>()
314+
.join(", ")
315+
);
316+
return Ok(ExecResult::err(msg, 3));
317+
}
318+
};
310319

311320
// Compile the filter, registering any --arg/--argjson variable names
312321
let var_names: Vec<&str> = var_bindings.iter().map(|(n, _)| n.as_str()).collect();
@@ -316,15 +325,19 @@ impl Builtin for Jq {
316325
} else {
317326
compiler.with_global_vars(var_names.iter().copied())
318327
};
319-
let filter = compiler.compile(modules).map_err(|errs| {
320-
Error::Execution(format!(
321-
"jq: compile error: {}",
322-
errs.into_iter()
323-
.map(|e| format!("{:?}", e))
324-
.collect::<Vec<_>>()
325-
.join(", ")
326-
))
327-
})?;
328+
let filter = match compiler.compile(modules) {
329+
Ok(f) => f,
330+
Err(errs) => {
331+
let msg = format!(
332+
"jq: compile error: {}\n",
333+
errs.into_iter()
334+
.map(|e| format!("{:?}", e))
335+
.collect::<Vec<_>>()
336+
.join(", ")
337+
);
338+
return Ok(ExecResult::err(msg, 3));
339+
}
340+
};
328341

329342
// Process input as JSON
330343
let mut output = String::new();
@@ -335,12 +348,16 @@ impl Builtin for Jq {
335348
vec![Val::from(serde_json::Value::Null)]
336349
} else if slurp {
337350
// -s flag: read all inputs into a single array
338-
let vals = Self::parse_json_values(input)?;
339-
vec![Val::from(serde_json::Value::Array(vals))]
351+
match Self::parse_json_values(input) {
352+
Ok(vals) => vec![Val::from(serde_json::Value::Array(vals))],
353+
Err(e) => return Ok(ExecResult::err(format!("{}\n", e), 5)),
354+
}
340355
} else {
341356
// Parse all JSON values from input (handles multi-line and NDJSON)
342-
let json_vals = Self::parse_json_values(input)?;
343-
json_vals.into_iter().map(Val::from).collect()
357+
match Self::parse_json_values(input) {
358+
Ok(json_vals) => json_vals.into_iter().map(Val::from).collect(),
359+
Err(e) => return Ok(ExecResult::err(format!("{}\n", e), 5)),
360+
}
344361
};
345362

346363
// Expose bashkit's shell env/variables to the process environment so
@@ -368,8 +385,12 @@ impl Builtin for Jq {
368385
let shared_inputs = RcIter::new(inputs_to_process.into_iter().map(Ok::<Val, String>));
369386

370387
for jaq_input in &shared_inputs {
371-
let jaq_input: Val =
372-
jaq_input.map_err(|e| Error::Execution(format!("jq: input error: {}", e)))?;
388+
let jaq_input: Val = match jaq_input {
389+
Ok(v) => v,
390+
Err(e) => {
391+
return Ok(ExecResult::err(format!("jq: input error: {}\n", e), 5));
392+
}
393+
};
373394

374395
// Run the filter, passing any --arg/--argjson variable values
375396
let var_vals: Vec<Val> = var_bindings
@@ -432,10 +453,10 @@ impl Builtin for Jq {
432453
}
433454
}
434455
Err(e) => {
435-
return Err(Error::Execution(format!(
436-
"jq: output error: {}",
437-
e
438-
)));
456+
return Ok(ExecResult::err(
457+
format!("jq: output error: {}\n", e),
458+
5,
459+
));
439460
}
440461
}
441462
}
@@ -447,10 +468,10 @@ impl Builtin for Jq {
447468
output.push('\n');
448469
}
449470
Err(e) => {
450-
return Err(Error::Execution(format!(
451-
"jq: output error: {}",
452-
e
453-
)));
471+
return Ok(ExecResult::err(
472+
format!("jq: output error: {}\n", e),
473+
5,
474+
));
454475
}
455476
}
456477
} else if tab_indent {
@@ -472,16 +493,16 @@ impl Builtin for Jq {
472493
output.push('\n');
473494
}
474495
Err(e) => {
475-
return Err(Error::Execution(format!(
476-
"jq: output error: {}",
477-
e
478-
)));
496+
return Ok(ExecResult::err(
497+
format!("jq: output error: {}\n", e),
498+
5,
499+
));
479500
}
480501
}
481502
}
482503
}
483504
Err(e) => {
484-
return Err(Error::Execution(format!("jq: runtime error: {:?}", e)));
505+
return Ok(ExecResult::err(format!("jq: runtime error: {:?}\n", e), 5));
485506
}
486507
}
487508
}
@@ -598,6 +619,52 @@ mod tests {
598619
Ok(result.stdout)
599620
}
600621

622+
async fn run_jq_result(filter: &str, input: &str) -> Result<ExecResult> {
623+
let jq = Jq;
624+
let fs = Arc::new(InMemoryFs::new());
625+
let mut vars = HashMap::new();
626+
let mut cwd = PathBuf::from("/");
627+
let args = vec![filter.to_string()];
628+
629+
let ctx = Context {
630+
args: &args,
631+
env: &HashMap::new(),
632+
variables: &mut vars,
633+
cwd: &mut cwd,
634+
fs,
635+
stdin: Some(input),
636+
#[cfg(feature = "http_client")]
637+
http_client: None,
638+
#[cfg(feature = "git")]
639+
git_client: None,
640+
};
641+
642+
jq.execute(ctx).await
643+
}
644+
645+
async fn run_jq_result_with_args(args: &[&str], input: &str) -> Result<ExecResult> {
646+
let jq = Jq;
647+
let fs = Arc::new(InMemoryFs::new());
648+
let mut vars = HashMap::new();
649+
let mut cwd = PathBuf::from("/");
650+
let args: Vec<String> = args.iter().map(|s| s.to_string()).collect();
651+
652+
let ctx = Context {
653+
args: &args,
654+
env: &HashMap::new(),
655+
variables: &mut vars,
656+
cwd: &mut cwd,
657+
fs,
658+
stdin: Some(input),
659+
#[cfg(feature = "http_client")]
660+
http_client: None,
661+
#[cfg(feature = "git")]
662+
git_client: None,
663+
};
664+
665+
jq.execute(ctx).await
666+
}
667+
601668
#[tokio::test]
602669
async fn test_jq_raw_output() {
603670
let result = run_jq_with_args(&["-r", ".name"], r#"{"name":"test"}"#)
@@ -658,15 +725,15 @@ mod tests {
658725
let close = "]".repeat(depth);
659726
let input = format!("{open}1{close}");
660727

661-
let result = run_jq(".", &input).await;
728+
let result = run_jq_result(".", &input).await.unwrap();
662729
assert!(
663-
result.is_err(),
730+
result.exit_code != 0,
664731
"deeply nested JSON arrays must be rejected"
665732
);
666-
let err = result.unwrap_err().to_string();
667733
assert!(
668-
err.contains("nesting too deep") || err.contains("recursion limit"),
669-
"error should mention nesting or recursion limit: {err}"
734+
result.stderr.contains("nesting too deep") || result.stderr.contains("recursion limit"),
735+
"error should mention nesting or recursion limit: {}",
736+
result.stderr
670737
);
671738
}
672739

@@ -680,15 +747,15 @@ mod tests {
680747
input = format!(r#"{{"a":{input}}}"#);
681748
}
682749

683-
let result = run_jq(".", &input).await;
750+
let result = run_jq_result(".", &input).await.unwrap();
684751
assert!(
685-
result.is_err(),
752+
result.exit_code != 0,
686753
"deeply nested JSON objects must be rejected"
687754
);
688-
let err = result.unwrap_err().to_string();
689755
assert!(
690-
err.contains("nesting too deep") || err.contains("recursion limit"),
691-
"error should mention nesting or recursion limit: {err}"
756+
result.stderr.contains("nesting too deep") || result.stderr.contains("recursion limit"),
757+
"error should mention nesting or recursion limit: {}",
758+
result.stderr
692759
);
693760
}
694761

@@ -962,22 +1029,38 @@ mod tests {
9621029

9631030
#[tokio::test]
9641031
async fn test_jq_invalid_json_input() {
965-
let result = run_jq(".", "not valid json");
966-
assert!(result.await.is_err(), "invalid JSON input should error");
1032+
let result = run_jq_result(".", "not valid json").await.unwrap();
1033+
assert!(
1034+
result.exit_code != 0,
1035+
"invalid JSON input should have non-zero exit"
1036+
);
1037+
assert!(
1038+
result.stderr.contains("jq:"),
1039+
"should have jq error in stderr"
1040+
);
9671041
}
9681042

9691043
#[tokio::test]
9701044
async fn test_jq_invalid_filter_syntax() {
971-
let result = run_jq(".[", r#"{"a":1}"#);
972-
assert!(result.await.is_err(), "invalid filter should error");
1045+
let result = run_jq_result(".[", r#"{"a":1}"#).await.unwrap();
1046+
assert!(
1047+
result.exit_code != 0,
1048+
"invalid filter should have non-zero exit"
1049+
);
1050+
assert!(
1051+
result.stderr.contains("jq:"),
1052+
"should have jq error in stderr"
1053+
);
9731054
}
9741055

9751056
#[tokio::test]
9761057
async fn test_jq_invalid_argjson_value() {
977-
let result = run_jq_with_args(&["--argjson", "x", "not json", "-n", "$x"], "");
1058+
let result = run_jq_result_with_args(&["--argjson", "x", "not json", "-n", "$x"], "")
1059+
.await
1060+
.unwrap();
9781061
assert!(
979-
result.await.is_err(),
980-
"invalid JSON for --argjson should error"
1062+
result.exit_code != 0,
1063+
"invalid JSON for --argjson should have non-zero exit"
9811064
);
9821065
}
9831066

‎crates/bashkit/tests/issue_277_test.rs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//! Regression test for #277: jq stderr output cannot be suppressed via 2>/dev/null
2+
3+
use bashkit::Bash;
4+
5+
#[tokio::test]
6+
async fn issue_277_jq_stderr_suppressed() {
7+
let mut bash = Bash::new();
8+
let r = bash
9+
.exec(r#"echo "not json" | jq '.foo' 2>/dev/null; echo "exit=$?""#)
10+
.await
11+
.unwrap();
12+
// stderr should be suppressed by 2>/dev/null; stdout should only contain exit code line
13+
assert!(
14+
!r.stdout.contains("error"),
15+
"jq error should be suppressed, stdout={:?}",
16+
r.stdout
17+
);
18+
assert!(
19+
r.stdout.contains("exit="),
20+
"should see exit code echo, stdout={:?}",
21+
r.stdout
22+
);
23+
}

0 commit comments

Comments
 (0)