From d37a6b76a465a1fc62372ffe51d4cf4bb5233633 Mon Sep 17 00:00:00 2001 From: SIDDARTHA REDDY <75976672+SIDDARTHAREDDY8@users.noreply.github.com> Date: Thu, 22 Oct 2026 13:25:58 -0400 Subject: [PATCH] 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. --- rust/lancedb/src/job.rs | 80 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 9 deletions(-) diff --git a/rust/lancedb/src/job.rs b/rust/lancedb/src/job.rs index 711a73033..6865e59ee 100644 --- a/rust/lancedb/src/job.rs +++ b/rust/lancedb/src/job.rs @@ -101,21 +101,19 @@ impl TerminalResult { fn decode(self) -> Result { 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 = Arc Result + Send + Sync>; enum JobInner { @@ -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::().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::().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:?}" + ); + } }