feat: actually using namespace error variants by hamersaw · Pull Request #6275 · lance-format/lance

hamersaw · 2026-03-24T13:53:37Z

In the rust SDK we have a NamespaceError that contains a message and an error code. Additionally, in both the Java and python SDKs we have logic to parse this type (and error codes) to produce native Java exceptions. The problem, is that in the namespace-impls we are never using them. Therefore the rust NamespaceError type is never used and the Java / python SDK conversion code the parse and throw native errors is essentially dead code. In this PR, we update the namespace-impls to actually throw the correct error types.

Closes #6240

github-actions · 2026-03-24T13:54:54Z

PR Review

Clean, mechanical migration from generic Error::namespace_source to typed NamespaceError variants. The error categorization choices look correct overall. Two issues worth addressing:

P1: REST client maps all HTTP errors to `NamespaceError::Internal`

In rest.rs, every non-success HTTP response is now mapped to NamespaceError::Internal. This defeats the purpose of this PR — if the REST server returns a structured error with an error code (e.g., 404 → TableNotFound, 409 → TableAlreadyExists), the client-side code should try to parse and preserve that code rather than collapsing everything to Internal. The Java/Python SDK parsing logic you mention in the description would need the correct error codes to come through.

Consider either:

Parsing the response body for a structured error (if the REST API returns error codes), or
At minimum, mapping HTTP status codes to appropriate variants (404 → TableNotFound/NamespaceNotFound, 409 → TableAlreadyExists/ConcurrentModification, 400 → InvalidInput, 403 → PermissionDenied, 401 → Unauthenticated, 503 → ServiceUnavailable).

If this is intentionally deferred, a TODO comment would be helpful.

Minor: Verbose `lance_core::Error::from(NamespaceError::... { ... })` pattern in `.map_err`

Several .map_err closures use the explicit lance_core::Error::from(NamespaceError::Foo { message: ... }) form, while others use the more concise NamespaceError::Foo { ... }.into(). Both work, but the inconsistency is noticeable. The .into() form is shorter — consider standardizing on it throughout.

Tests are updated to match the new error messages. The variant choices (TableNotFound, InvalidInput, NamespaceAlreadyExists, ConcurrentModification, etc.) all look semantically appropriate. LGTM otherwise.

hamersaw · 2026-03-24T13:57:48Z

Mentioning parallel work that updates to throw on TableNodeFound - #6267

hamersaw · 2026-03-24T14:07:46Z

Another effort to add TableNotFound - #6252

codecov · 2026-03-24T14:26:31Z

Codecov Report

❌ Patch coverage is 9.97758% with 803 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance-namespace-impls/src/dir/manifest.rs	2.67%	252 Missing and 3 partials ⚠️
rust/lance-namespace-impls/src/dir.rs	6.34%	192 Missing ⚠️
rust/lance-namespace-impls/src/rest.rs	22.08%	119 Missing and 8 partials ⚠️
...ust/lance-namespace-impls/src/credentials/azure.rs	0.00%	103 Missing ⚠️
rust/lance-namespace-impls/src/credentials/gcp.rs	12.30%	57 Missing ⚠️
rust/lance-namespace-impls/src/credentials/aws.rs	0.00%	31 Missing ⚠️
rust/lance-namespace/src/error.rs	32.00%	16 Missing and 1 partial ⚠️
rust/lance-namespace-impls/src/rest_adapter.rs	55.55%	12 Missing ⚠️
rust/lance-namespace-impls/src/credentials.rs	0.00%	9 Missing ⚠️

📢 Thoughts on this report? Let us know!

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Replace generic lance_core::Error variants (invalid_input_source, io_source, not_supported_source, etc.) with NamespaceError variants throughout the namespace-impls crate. This ensures errors flow through the LanceError::Namespace path and get properly converted to lance_namespace.errors Python exceptions with correct error codes. Also fix table_exists fallback to only fall through to directory check for single-level IDs, matching the existing describe_table pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jackye1995

thanks, long overdue. I think the title should be feat or fix, not chore since it changes user experience.

…-error-variants

wjones127

This seems good, but I'm noting there is a pre-existing issue where the NamespaceError has Display implementations and message fields. Most of the time, they seem to duplicate information. I've noted two cases of this, but here are several more. I'd consider cleaning those up, either here or in a follow up.

wjones127 · 2026-03-25T18:51:01Z

+            None => Err(NamespaceError::NamespaceNotFound {
+                message: format!("Namespace '{}' not found", object_id),
+            }
+            .into()),


suggestion: This message duplicates what is in the Display implementation for this error:

lance/rust/lance-namespace/src/error.rs

Lines 180 to 182 in baa13ab

/// The specified namespace does not exist.

#[snafu(display("Namespace not found: {message}"))]

NamespaceNotFound { message: String },

The error will print as:

Namespace not found: Namespace '{name}' not found

Consider just putting the quoted object id as the message:

Suggested change

None => Err(NamespaceError::NamespaceNotFound {

message: format!("Namespace '{}' not found", object_id),

}

.into()),

Err(NamespaceError::NamespaceNotFound {

message: format!("'{}'", object_id)

}

.into())

Then the message would be like:

Namespace not found: '{name}'

You could also change the error itself to table a table name instead of a custom message.

Great call out. I updated all of these that I could find. But just like replacing all the errors, I'm sure I missed one and will have to come back and fix it 😆

wjones127 · 2026-03-25T18:53:01Z

+            return Err(NamespaceError::NamespaceAlreadyExists {
+                message: format!("Namespace '{}' already exists", object_id),
+            }
+            .into());


issue(non-blocking): similar problem here: message duplicates the Display implementations of NamespaceError::NamespaceAlreadyExists:

lance/rust/lance-namespace/src/error.rs

Lines 184 to 186 in baa13ab

/// A namespace with this name already exists.

#[snafu(display("Namespace already exists: {message}"))]

NamespaceAlreadyExists { message: String },

The Display impl on NamespaceError variants already adds context like "Namespace not found: " or "Table already exists: ", so messages should contain just the entity identifier rather than repeating the variant semantics (e.g. "Namespace 'foo' not found"). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The REST server was serializing ns_err.to_string() (which includes the Display prefix) as the error message. The client then wrapped that full string back into a new NamespaceError, causing doubled prefixes like "Namespace not empty: Namespace not empty: ...". Use the new message() accessor to send just the inner message without the Display prefix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

In the rust SDK we have a `NamespaceError` that contains a message and an error code. Additionally, in both the Java and python SDKs we have logic to parse this type (and error codes) to produce native Java exceptions. The problem, is that in the `namespace-impls` we are never using them. Therefore the rust `NamespaceError` type is never used and the Java / python SDK conversion code the parse and throw native errors is essentially dead code. In this PR, we update the `namespace-impls` to actually throw the correct error types. Closes lance-format#6240 --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

using namespace error variants to parse error codes

69ed280

github-actions Bot added the chore label Mar 24, 2026

hamersaw mentioned this pull request Mar 24, 2026

fix: handle JNI RuntimeException in describeTable for Spark 4.0+ tableExists lance-format/lance-spark#326

Merged

7 tasks

updating tests to ensure errors are actually thrown

388598e

github-actions Bot added python java labels Mar 24, 2026

hamersaw and others added 2 commits March 24, 2026 09:58

fix: collapse nested if-let to satisfy clippy

dbc4bb3

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jackye1995 approved these changes Mar 25, 2026

View reviewed changes

hamersaw changed the title ~~chore: actually using namespace error variants~~ feat: actually using namespace error variants Mar 25, 2026

github-actions Bot added the enhancement New feature or request label Mar 25, 2026

hamersaw added 3 commits March 25, 2026 12:02

parsing rest error codes to determine NamespaceError code

e551eaa

Merge remote-tracking branch 'upstream/main' into chore/use-namespace…

71a84fa

…-error-variants

updating main merge to use NamespaceError variants

922f120

wjones127 approved these changes Mar 25, 2026

View reviewed changes

wjones127 mentioned this pull request Mar 25, 2026

fix: namespace_exists/table_exists raise NamespaceNotFoundError/TableNotFoundError #6252

Closed

hamersaw and others added 3 commits March 25, 2026 15:24

Merge branch 'main' into chore/use-namespace-error-variants

e937c15

wjones127 merged commit 3a82648 into lance-format:main Mar 26, 2026
28 checks passed

hamersaw deleted the chore/use-namespace-error-variants branch March 26, 2026 16:49

hamersaw mentioned this pull request Apr 2, 2026

fix: use typed NamespaceError::TableNotFound in DirectoryNamespace instead of plain strings #6267

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: actually using namespace error variants#6275

feat: actually using namespace error variants#6275
wjones127 merged 10 commits intolance-format:mainfrom
hamersaw:chore/use-namespace-error-variants

hamersaw commented Mar 24, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Mar 24, 2026

Uh oh!

hamersaw commented Mar 24, 2026

Uh oh!

hamersaw commented Mar 24, 2026

Uh oh!

codecov Bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

jackye1995 left a comment

Uh oh!

wjones127 left a comment

Uh oh!

wjones127 Mar 25, 2026

Uh oh!

wjones127 Mar 25, 2026

Uh oh!

hamersaw Mar 26, 2026

Uh oh!

wjones127 Mar 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	/// The specified namespace does not exist.
	#[snafu(display("Namespace not found: {message}"))]
	NamespaceNotFound { message: String },

	/// A namespace with this name already exists.
	#[snafu(display("Namespace already exists: {message}"))]
	NamespaceAlreadyExists { message: String },

Conversation

hamersaw commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Mar 24, 2026

PR Review

P1: REST client maps all HTTP errors to NamespaceError::Internal

Minor: Verbose lance_core::Error::from(NamespaceError::... { ... }) pattern in .map_err

Uh oh!

hamersaw commented Mar 24, 2026

Uh oh!

hamersaw commented Mar 24, 2026

Uh oh!

codecov Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jackye1995 left a comment

Choose a reason for hiding this comment

Uh oh!

wjones127 left a comment

Choose a reason for hiding this comment

Uh oh!

wjones127 Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

wjones127 Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

hamersaw Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

wjones127 Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hamersaw commented Mar 24, 2026 •

edited

Loading

P1: REST client maps all HTTP errors to `NamespaceError::Internal`

Minor: Verbose `lance_core::Error::from(NamespaceError::... { ... })` pattern in `.map_err`

codecov Bot commented Mar 24, 2026 •

edited

Loading