Skip to content

fix(rust): return NotFound on driver not found instead of Internal on Windows - #4971

Merged
lidavidm merged 4 commits into
apache:mainfrom
amoeba:windows-driver-not-found-repro
Oct 28, 2026
Merged

lidavidm merged 4 commits into
apache:mainfrom
amoeba:windows-driver-not-found-repro

Conversation

@amoeba

@amoeba amoeba commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

The Rust driver manager returns an inconsistent error class across platforms. On non-Windows platforms, trying to load a driver that doesn't exist is handled and we return NotFound. On Windows, don't catch a failed load and just pass it upwards which results in an Internal error. I don't think there's any good reason to return different errors for the same behavior across platforms.

There's a TODO about this: https://github.com/apache/arrow-adbc/blob/main/rust/driver_manager/src/search.rs#L527.

This PR syncs the behavior between non-Windows and Windows platforms in the Rust driver manager and adds a regression test in the Javascript bindings. The new behavior is tested by updating the existing test_load_from_name_missing_driver test to not only verify we return a non-empty error but that we return a Driver not found error with detail.

@amoeba
amoeba requested review from lidavidm and zeroshade October 27, 2026 22:33
@amoeba
amoeba marked this pull request as ready for review October 27, 2026 22:33
@amoeba
amoeba requested a review from wjones127 as a code owner October 27, 2026 22:33
lidavidm pushed a commit that referenced this pull request Oct 28, 2026
Adds some basic test coverage for loading drivers by name/path. I found
these gaps while working on
#4971.

@lidavidm lidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Maybe it's worth trying to do the driver manager validation suite...

@lidavidm
lidavidm merged commit b4fc356 into apache:main Oct 28, 2026
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants