feat: return Unprocessable error while expected error happened #5347
Conversation
Signed-off-by: Xuanwo <[email protected]>
Signed-off-by: Xuanwo <[email protected]>
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
I agree that Error::Index is not terribly useful here but I wonder if we need a new error type?
rust/lance-core/src/error.rs
Outdated
| #[snafu(display("Precondition failed: {message}, {location}"))] | ||
| Precondition { message: String, location: Location }, |
There was a problem hiding this comment.
Is Precondition meaningfully different from InvalidInput?
There was a problem hiding this comment.
Yes, Precondition here follows the same pattern as HTTP Status 412 Precondition, used by many services to indicate that the request itself is valid but the condition is not met. In this case, the dataset doesn’t have enough data. I wish this error could be used by users to identify these cases instead of bypassing real invalid input errors.
There was a problem hiding this comment.
What about just saying it is unsupported? since this after all is us not able to support building/refreshing the index with this amount of data.
|
cc @BubbleCal, maybe you will be interested in this PR. |
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
My only nit with 412 is that it is normally used where the caller provides the preconditions (e.g. if-... headers). I think HTTP 422 (unprocessable) might be slightly more accurate. However, we are well into bike-shedding at this point so I'll trust your judgement.
BubbleCal
left a comment
BubbleCal
left a comment
There was a problem hiding this comment.
I think we do error mapping somewhere else (e.g. lancedb), we may need to add a new map for Precondition
Oh, I learned a new HTTP status code. After some thought, I think |
Xuanwo
changed the title
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This PR intends to return Unprocessable error while expected error happened. In this way, our users can print helpful message to users instead of just return the Index error.
This PR was primarily authored with Codex using GPT-5-Codex and then hand-reviewed by me. I AM responsible for every change made in this PR. I aimed to keep it aligned with our goals, though I may have missed minor issues. Please flag anything that feels off, I'll fix it quickly.