fix(rust): return typed errors instead of panicking in Bedrock embedding path#3512
Merged
Merged
Conversation
Contributor
Author
|
The two failing NodeJS example jobs are HuggingFace HTTP 429 (rate-limit) errors while downloading |
Contributor
Author
|
Friendly ping! Let me know if any changes are needed. |
Contributor
Author
|
Hi, friendly ping, please let me know if any changes are needed. |
wjones127
approved these changes
Jun 17, 2026
wjones127
left a comment
Contributor
There was a problem hiding this comment.
This all looks good to me. It looks like you might need to rebase this PR before this will be passing tests and ready to merge.
Ar-maan05
force-pushed
the
fix/bedrock-panic-3506
branch
from
June 17, 2026 21:33
a807f5d to
544909d
Compare
Contributor
Author
|
@wjones127 rebased onto main, CI should be green now. Thanks for the review! I've really enjoyed contributing here, happy to pick up other open issues if any would be helpful. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3506
Problem
The Bedrock embedding compute path (
rust/lancedb/src/embeddings/bedrock.rs) panics instead of returning a typed error in several places:serde_json::to_vec(&request_body).unwrap(): request serialization.block_in_place(...).unwrap(): the AWSinvoke_modelsend result; any API error terminates the worker instead of propagating.v.as_f64().unwrap() as f32: panics on non-numeric values in the returned embedding array.Handle::current()+block_in_placeassume a multi-threaded Tokio runtime and panic when that assumption does not hold (no runtime, or a current-thread runtime).Malformed payloads, non-numeric embedding values, or an incompatible runtime should surface as typed errors and never panic.
Fix
Error::Runtimevia?.invoke_modelsend error toError::Runtimeinstead ofunwrap.json_array_to_f32helper that converts the response array toVec<f32>, returningError::Runtimefor a missing/non-array field or a non-numeric element (used by both the Titan and Cohere paths).current_multi_thread_handle()(Handle::try_current()+ aRuntimeFlavor::CurrentThreadguard) so an absent or incompatible runtime returns a typed error rather than panicking inblock_in_place.Scope note: the sibling
openai.rsprovider uses the sameblock_in_place+block_onbridge, so the bridge pattern itself is kept; this change only removes the panic paths that are specific to the Bedrock provider.Testing
Added 6 unit tests (no AWS credentials required):
json_array_to_f32: valid numbers, non-array payload, and non-numeric element.current_multi_thread_handle: errors with no runtime, errors on a current-thread runtime, and succeeds on a multi-threaded runtime.All pass;
cargo fmtandcargo clippyclean. Build/test with--features bedrock,lance/protoc.