diff --git a/crates/cli/src/commands/table/mod.rs b/crates/cli/src/commands/table/mod.rs index 5301818..1523c50 100644 --- a/crates/cli/src/commands/table/mod.rs +++ b/crates/cli/src/commands/table/mod.rs @@ -406,10 +406,10 @@ fn properties(values: Vec) -> Result> { Ok(result) } fn require_string(body: &Value, field: &str) -> Result<()> { - if !body + if body .get(field) .and_then(Value::as_str) - .is_some_and(|s| !s.trim().is_empty()) + .is_none_or(|s| s.trim().is_empty()) { return Err(Error::Config(format!("Request requires nonempty {field}"))); } @@ -577,10 +577,10 @@ fn prepare_table(command: TableCommands) -> Result { { return Err(Error::Config("Standard updates use Iceberg requirements; version/location guards require new-metadata-location".into())); } - if !body + if body .get("requirements") .and_then(Value::as_array) - .is_some_and(|v| !v.is_empty()) + .is_none_or(Vec::is_empty) { return Err(Error::Config( "Standard commit requires explicit Iceberg requirements".into(), diff --git a/crates/s3/src/admin/catalog.rs b/crates/s3/src/admin/catalog.rs index c021a84..3202332 100644 --- a/crates/s3/src/admin/catalog.rs +++ b/crates/s3/src/admin/catalog.rs @@ -302,8 +302,7 @@ impl AdminClient { serde_json::from_slice(&bytes) .map_err(|_| Error::General("Invalid catalog JSON response".into()))? }; - if !value.is_object() - && !(request.operation == Op::MaintenanceConfigShow && value.is_null()) + if !(value.is_object() || request.operation == Op::MaintenanceConfigShow && value.is_null()) { return Err(Error::General("Catalog response must be an object".into())); } diff --git a/crates/s3/src/select.rs b/crates/s3/src/select.rs index c46dac6..23f3ca2 100644 --- a/crates/s3/src/select.rs +++ b/crates/s3/src/select.rs @@ -295,7 +295,9 @@ fn map_select_initial_error( match &err { SdkError::ServiceError(se) => { let code = resolve_http_service_error_code(se.err(), se.raw()); - classify_aws_code(code, &err.to_string()) + let fallback = err.to_string(); + let message = se.err().message().unwrap_or(&fallback); + classify_aws_code(code, message) } SdkError::TimeoutError(_) => Error::Network("Request timeout".to_string()), SdkError::DispatchFailure(e) => Error::Network(format!("Network dispatch error: {e:?}")), @@ -312,7 +314,9 @@ fn map_select_stream_error( match &err { SdkError::ServiceError(se) => { let code = resolve_event_stream_error_code(se.err(), se.raw()); - classify_aws_code(code, &err.to_string()) + let fallback = err.to_string(); + let message = se.err().message().unwrap_or(&fallback); + classify_aws_code(code, message) } SdkError::TimeoutError(_) => Error::Network("Request timeout".to_string()), SdkError::DispatchFailure(e) => Error::Network(format!("Network dispatch error: {e:?}")), @@ -324,6 +328,9 @@ fn map_select_stream_error( fn classify_aws_code(code: Option<&str>, text: &str) -> Error { let c = code.filter(|s| !s.is_empty()); + if text.contains("NotImplemented") && c != Some("NotImplemented") { + return Error::UnsupportedFeature("The backend does not support S3 Select.".to_string()); + } match c { Some("NoSuchKey") => Error::NotFound("Object not found".to_string()), Some("NoSuchBucket") => Error::NotFound("Bucket not found".to_string()), @@ -331,15 +338,29 @@ fn classify_aws_code(code: Option<&str>, text: &str) -> Error { Some("NotImplemented") => { Error::UnsupportedFeature("The backend does not support S3 Select.".to_string()) } - Some("InvalidArgument") => Error::General(format!("Invalid S3 Select request: {text}")), - Some(_) if text.contains("NotImplemented") => { - Error::UnsupportedFeature("The backend does not support S3 Select.".to_string()) - } - Some(_) => Error::General(text.to_string()), + Some("SlowDown" | "Busy") => Error::Network(service_error_detail(c, text)), + Some("InvalidArgument") => Error::General(format!( + "Invalid S3 Select request: {}", + service_error_detail(c, text) + )), + Some("UnsupportedScanRangeInput") => Error::General(service_error_detail(c, text)), + Some(_) => Error::General(service_error_detail(c, text)), None => classify_aws_code_missing_metadata(text), } } +fn service_error_detail(code: Option<&str>, text: &str) -> String { + let text = text.trim(); + match ( + code, + text.is_empty() || text.eq_ignore_ascii_case("service error"), + ) { + (Some(code), true) => code.to_string(), + (Some(code), false) => format!("{code}: {text}"), + (None, _) => text.to_string(), + } +} + /// When the SDK did not surface `x-amz-error-code` / metadata, use minimal substring checks. fn classify_aws_code_missing_metadata(text: &str) -> Error { if text.contains("NotImplemented") { @@ -385,7 +406,32 @@ mod tests { #[test] fn classify_fallback_network() { let e = classify_aws_code(Some("SlowDown"), "rate limited"); - assert!(matches!(e, Error::General(_))); + assert!(matches!(e, Error::Network(message) if message == "SlowDown: rate limited")); + } + + #[test] + fn classify_busy_preserves_service_context() { + let e = classify_aws_code(Some("Busy"), "The service is unavailable. Try again later."); + assert!( + matches!(e, Error::Network(message) if message.contains("Busy") && message.contains("unavailable")) + ); + } + + #[test] + fn classify_unsupported_scan_range_preserves_service_context() { + let e = classify_aws_code( + Some("UnsupportedScanRangeInput"), + "Scan range queries are not supported on this type of object.", + ); + assert!( + matches!(e, Error::General(message) if message.contains("UnsupportedScanRangeInput") && message.contains("not supported")) + ); + } + + #[test] + fn classify_unsupported_scan_range_replaces_generic_service_text() { + let e = classify_aws_code(Some("UnsupportedScanRangeInput"), "service error"); + assert!(matches!(e, Error::General(message) if message == "UnsupportedScanRangeInput")); } #[test]