mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-23 12:12:27 +00:00
fix: gate remote job error construction behind the remote feature (#4233)
## Summary Fixes #4142: `lancedb` failed to compile without the `remote` feature. `TerminalResult::decode` in `rust/lancedb/src/job.rs` constructed the `Error::Http` variant unconditionally, but that variant is declared behind `#[cfg(feature = "remote")]` in `rust/lancedb/src/error.rs`. Since the crate's default feature set is empty, a plain `lancedb = "0.38"` dependency failed with: ``` error[E0599]: no variant named `Http` found for enum `error::Error` --> rust/lancedb/src/job.rs:102:40 ``` The module itself is not remote-only (it carries local job support), so instead of gating `pub mod job`, both `Error::Http` constructions now go through a small `remote_job_error` helper that resolves to `Error::Http` with the `remote` feature and falls back to `Error::Runtime` without it. A `request_id` is only ever `Some` for remote jobs, so the fallback is unreachable in practice without the feature. ## Testing - Reproduced the reported failure on current `main`: `cargo check -p lancedb --no-default-features` failed with the exact E0599 errors from the issue before the change, and passes after it. - `cargo check -p lancedb --features remote` passes with no errors and no new warnings. - Added two regression tests in `rust/lancedb/src/job.rs` (`decode_missing_result_uses_feature_appropriate_error`, `decode_unparsable_result_uses_feature_appropriate_error`) asserting the error variant matches the feature configuration. The test code compiles under the fixed configuration (`cargo check --tests` passes), and the branching logic was executed standalone against both feature cfgs (3/3 green with and without `remote`). The full `cargo test` binary link could not complete on the small build VM available here (the linker stalled), so CI coverage of the new tests is appreciated. - `cargo fmt --all -- --check` is clean. ## Disclosure AI assistance was used while preparing this change (code search, editing, and verification commands were agent-driven); the diagnosis, fix design, and all test results above were verified by actually running the builds.
This commit is contained in:
+71
-9
@@ -101,21 +101,19 @@ impl TerminalResult {
|
||||
|
||||
fn decode<T: DeserializeOwned>(self) -> Result<T> {
|
||||
let value = self.value.ok_or_else(|| match &self.request_id {
|
||||
Some(request_id) => Error::Http {
|
||||
source: "successful typed job response did not contain a result".into(),
|
||||
request_id: request_id.clone(),
|
||||
status_code: None,
|
||||
},
|
||||
Some(request_id) => remote_job_error(
|
||||
request_id.clone(),
|
||||
"successful typed job response did not contain a result".to_string(),
|
||||
),
|
||||
None => Error::Runtime {
|
||||
message: "successful typed job did not contain a result".to_string(),
|
||||
},
|
||||
})?;
|
||||
serde_json::from_value(value).map_err(|error| match self.request_id {
|
||||
Some(request_id) => Error::Http {
|
||||
source: format!("failed to parse typed job result: {error}").into(),
|
||||
Some(request_id) => remote_job_error(
|
||||
request_id,
|
||||
status_code: None,
|
||||
},
|
||||
format!("failed to parse typed job result: {error}"),
|
||||
),
|
||||
None => Error::Runtime {
|
||||
message: format!("failed to parse typed job result: {error}"),
|
||||
},
|
||||
@@ -123,6 +121,28 @@ impl TerminalResult {
|
||||
}
|
||||
}
|
||||
|
||||
/// Build the error for a failed remote typed-job result.
|
||||
///
|
||||
/// The `Error::Http` variant is gated behind the `remote` feature, so without
|
||||
/// it we fall back to `Error::Runtime`. A `request_id` is only ever `Some`
|
||||
/// for remote jobs, making the fallback unreachable in practice without the
|
||||
/// feature.
|
||||
#[cfg(feature = "remote")]
|
||||
fn remote_job_error(request_id: String, source: String) -> Error {
|
||||
Error::Http {
|
||||
source: source.into(),
|
||||
request_id,
|
||||
status_code: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "remote"))]
|
||||
fn remote_job_error(request_id: String, source: String) -> Error {
|
||||
Error::Runtime {
|
||||
message: format!("{source} (request_id={request_id})"),
|
||||
}
|
||||
}
|
||||
|
||||
type ResultDecoder<T> = Arc<dyn Fn(TerminalResult) -> Result<T> + Send + Sync>;
|
||||
|
||||
enum JobInner<T> {
|
||||
@@ -634,4 +654,46 @@ mod tests {
|
||||
assert!(matches!(job.wait().await, Err(Error::JobCancelled { .. })));
|
||||
assert_eq!(job.status().await.unwrap(), "cancelled");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn decode_missing_result_uses_feature_appropriate_error() {
|
||||
// A `request_id` is only `Some` for remote jobs. The error for a
|
||||
// missing result must use `Error::Http` with the `remote` feature and
|
||||
// fall back to `Error::Runtime` without it (where `Error::Http`
|
||||
// does not exist). See https://github.com/lancedb/lancedb/issues/4142.
|
||||
let result = TerminalResult {
|
||||
value: None,
|
||||
request_id: Some("req-1".to_string()),
|
||||
};
|
||||
let err = result.decode::<Value>().unwrap_err();
|
||||
#[cfg(feature = "remote")]
|
||||
assert!(
|
||||
matches!(err, Error::Http { .. }),
|
||||
"expected Error::Http, got {err:?}"
|
||||
);
|
||||
#[cfg(not(feature = "remote"))]
|
||||
assert!(
|
||||
matches!(err, Error::Runtime { .. }),
|
||||
"expected Error::Runtime, got {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn decode_unparsable_result_uses_feature_appropriate_error() {
|
||||
let result = TerminalResult {
|
||||
value: Some(Value::String("not a u64".to_string())),
|
||||
request_id: Some("req-1".to_string()),
|
||||
};
|
||||
let err = result.decode::<u64>().unwrap_err();
|
||||
#[cfg(feature = "remote")]
|
||||
assert!(
|
||||
matches!(err, Error::Http { .. }),
|
||||
"expected Error::Http, got {err:?}"
|
||||
);
|
||||
#[cfg(not(feature = "remote"))]
|
||||
assert!(
|
||||
matches!(err, Error::Runtime { .. }),
|
||||
"expected Error::Runtime, got {err:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user