Skip to content

Commit d13d206

Browse files
committed
fix(sheets): report error for invalid --json-values instead of silent empty append
parse_append_args() used unwrap_or_default() when JSON parsing failed, silently converting invalid input into an empty Vec. This caused +append --json-values with malformed JSON to append an empty row without any error message, making debugging very difficult. - Change parse_append_args() to return Result<AppendConfig, GwsError> - Surface the serde_json error message with guidance on expected format - Add #[derive(Debug)] to AppendConfig for Result unwrap in tests - Add tests for valid JSON and invalid JSON error paths
1 parent 6e63960 commit d13d206

1 file changed

Lines changed: 42 additions & 11 deletions

File tree

src/helpers/sheets.rs

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ TIPS:
102102
) -> Pin<Box<dyn Future<Output = Result<bool, GwsError>> + Send + 'a>> {
103103
Box::pin(async move {
104104
if let Some(matches) = matches.subcommand_matches("+append") {
105-
let config = parse_append_args(matches);
105+
let config = parse_append_args(matches)?;
106106
let (params_str, body_str, scopes) = build_append_request(&config, doc)?;
107107

108108
let scope_strs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
@@ -257,6 +257,7 @@ fn build_read_request(
257257
/// Configuration for appending values to a spreadsheet.
258258
///
259259
/// Holds the parsed arguments for the `+append` subcommand.
260+
#[derive(Debug)]
260261
pub struct AppendConfig {
261262
/// The ID of the spreadsheet to append to.
262263
pub spreadsheet_id: String,
@@ -267,7 +268,8 @@ pub struct AppendConfig {
267268
/// Parses arguments for the `+append` command.
268269
///
269270
/// Supports both `--values` (single row) and `--json-values` (single or multi-row).
270-
pub fn parse_append_args(matches: &ArgMatches) -> AppendConfig {
271+
/// Returns a validation error if `--json-values` contains invalid JSON.
272+
pub fn parse_append_args(matches: &ArgMatches) -> Result<AppendConfig, GwsError> {
271273
let values = if let Some(json_str) = matches.get_one::<String>("json-values") {
272274
// Try parsing as array-of-arrays (multi-row) first
273275
if let Ok(parsed) = serde_json::from_str::<Vec<Vec<String>>>(json_str) {
@@ -276,21 +278,20 @@ pub fn parse_append_args(matches: &ArgMatches) -> AppendConfig {
276278
// Single flat array — treat as one row
277279
vec![parsed]
278280
} else {
279-
eprintln!(
280-
"Warning: --json-values is not valid JSON; expected an array or array-of-arrays"
281-
);
282-
Vec::new()
281+
return Err(GwsError::Validation(format!(
282+
"--json-values is not valid JSON: expected an array like '[\"a\",\"b\"]' or array-of-arrays like '[[\"a\",\"b\"],[\"c\",\"d\"]]'."
283+
)));
283284
}
284285
} else if let Some(values_str) = matches.get_one::<String>("values") {
285286
vec![values_str.split(',').map(|s| s.to_string()).collect()]
286287
} else {
287288
Vec::new()
288289
};
289290

290-
AppendConfig {
291+
Ok(AppendConfig {
291292
spreadsheet_id: matches.get_one::<String>("spreadsheet").unwrap().clone(),
292293
values,
293-
}
294+
})
294295
}
295296

296297
/// Configuration for reading values from a spreadsheet.
@@ -395,7 +396,7 @@ mod tests {
395396
#[test]
396397
fn test_parse_append_args_values() {
397398
let matches = make_matches_append(&["test", "--spreadsheet", "123", "--values", "a,b,c"]);
398-
let config = parse_append_args(&matches);
399+
let config = parse_append_args(&matches).unwrap();
399400
assert_eq!(config.spreadsheet_id, "123");
400401
assert_eq!(config.values, vec![vec!["a", "b", "c"]]);
401402
}
@@ -409,7 +410,7 @@ mod tests {
409410
"--json-values",
410411
r#"["a","b","c"]"#,
411412
]);
412-
let config = parse_append_args(&matches);
413+
let config = parse_append_args(&matches).unwrap();
413414
assert_eq!(config.values, vec![vec!["a", "b", "c"]]);
414415
}
415416

@@ -422,7 +423,7 @@ mod tests {
422423
"--json-values",
423424
r#"[["Alice","100"],["Bob","200"]]"#,
424425
]);
425-
let config = parse_append_args(&matches);
426+
let config = parse_append_args(&matches).unwrap();
426427
assert_eq!(
427428
config.values,
428429
vec![vec!["Alice", "100"], vec!["Bob", "200"]]
@@ -447,6 +448,36 @@ mod tests {
447448
assert_eq!(values[1], json!(["Bob", "200"]));
448449
}
449450

451+
#[test]
452+
fn test_parse_append_args_json_valid() {
453+
let matches = make_matches_append(&[
454+
"test",
455+
"--spreadsheet",
456+
"123",
457+
"--json-values",
458+
r#"["a","b","c"]"#,
459+
]);
460+
let config = parse_append_args(&matches).unwrap();
461+
assert_eq!(config.values, vec![vec!["a", "b", "c"]]);
462+
}
463+
464+
#[test]
465+
fn test_parse_append_args_json_invalid_returns_error() {
466+
let matches = make_matches_append(&[
467+
"test",
468+
"--spreadsheet",
469+
"123",
470+
"--json-values",
471+
"not valid json",
472+
]);
473+
let err = parse_append_args(&matches).unwrap_err();
474+
let msg = err.to_string();
475+
assert!(
476+
msg.contains("--json-values is not valid JSON"),
477+
"expected JSON error message, got: {msg}"
478+
);
479+
}
480+
450481
#[test]
451482
fn test_parse_read_args() {
452483
let matches = make_matches_read(&["test", "--spreadsheet", "123", "--range", "A1:B2"]);

0 commit comments

Comments
 (0)