This is an automated email from the ASF dual-hosted git repository.
CurtHagenlocher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 075b302aba fix(csharp): align driver and manifest loading errors with
c++ (#4973)
075b302aba is described below
commit 075b302abae2db15c5586cd76224ee9423678163
Author: Bryce Mecum <[email protected]>
AuthorDate: Wed Oct 28 16:54:37 2026 -0700
fix(csharp): align driver and manifest loading errors with c++ (#4973)
This PR syncs the C# driver manager's failure behavior with the other
driver managers.
I noticed the driver manager implementations don't perfectly align with
respect to what errors they return for various driver and manifest
loading failures. I'm not sure we need to have every driver manager
return the exact same error class and detail but I noticed the C# driver
manager returns non-ADBC errors in some cases where the other driver
managers return an ADBC error with the underlying error wrapped. I think
at least being consistent about when the user gets an ADBC error or not
is a good starting place.
I had Opus 5.5 help me outline these cases, analyze other driver
managers, and produce this table that highlights the behavioral
difference in the C# driver manager:
| Case | C/C++ | Rust | C# |
|---|---|---|---|
| **Driver library given by path** (absolute or relative, with or
without extension) | | | |
| File doesn't exist | ADBC `NOT_FOUND` | ADBC `INTERNAL` |
`System.ArgumentException` (not an ADBC error) |
| File exists but can't be loaded (not a shared library, wrong
architecture) | ADBC `NOT_FOUND` | ADBC `INTERNAL` |
`System.DllNotFoundException` (not an ADBC error) |
| **Manifest given by path** (`.toml`, or a path without extension with
a `.toml` of the same name next to it) | | | |
| Manifest file doesn't exist | ADBC `INVALID_ARGUMENT` | ADBC
`INVALID_ARGUMENT` | ADBC `NOT_FOUND` |
| Manifest isn't valid TOML | ADBC `INVALID_ARGUMENT` | ADBC
`INVALID_ARGUMENT` | ADBC `INVALID_ARGUMENT` |
| Manifest has no `Driver.shared` key | ADBC `INVALID_ARGUMENT` | ADBC
`INVALID_ARGUMENT` | ADBC `INVALID_ARGUMENT` |
| Manifest has no `Driver.shared` entry for this platform | ADBC
`NOT_FOUND` | ADBC `INVALID_ARGUMENT` | ADBC `NOT_FOUND` |
| Manifest's driver library doesn't exist | ADBC `NOT_FOUND` | ADBC
`INTERNAL` | `System.ArgumentException` (not an ADBC error) |
| Manifest's driver library exists but can't be loaded | ADBC
`NOT_FOUND` | ADBC `INTERNAL` | `System.DllNotFoundException` (not an
ADBC error) |
| **Driver given by bare name** (found via search paths) | | | |
| Nothing found by that name | ADBC `NOT_FOUND` | ADBC `NOT_FOUND` |
ADBC `NOT_FOUND` |
| Manifest found, its driver library doesn't exist | ADBC `NOT_FOUND` |
ADBC `NOT_FOUND` | `System.ArgumentException` (not an ADBC error) |
| Manifest found, its driver library exists but can't be loaded | ADBC
`NOT_FOUND` | ADBC `NOT_FOUND` | `System.DllNotFoundException` (not an
ADBC error) |
The PR makes C# match C++ in the above cases. Technically this could be
marked as a breaking change though it's less likely that users depend on
error behaviors in the same way they depend on non-error behaviors.
---
.../src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs | 4 +-
.../DriverManager/AdbcDriverManager.cs | 67 ++++--
.../DriverManager/DriverNotFoundTests.cs | 229 +++++++++++++++++++++
.../DriverManager/TomlConnectionProfileTests.cs | 2 +-
4 files changed, 284 insertions(+), 18 deletions(-)
diff --git a/csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs
b/csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs
index 19b85e123a..57e96ef721 100644
--- a/csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs
+++ b/csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs
@@ -79,7 +79,9 @@ namespace Apache.Arrow.Adbc.C
IntPtr library = NativeLibrary.Load(file);
if (library == IntPtr.Zero)
{
- throw new ArgumentException("unable to load library",
nameof(file));
+ // Only reachable with the netstandard2.0 NativeLibrary shim;
the
+ // runtime's NativeLibrary.Load throws DllNotFoundException
itself.
+ throw new DllNotFoundException($"Unable to load shared library
'{file}'.");
}
try
diff --git a/csharp/src/Apache.Arrow.Adbc/DriverManager/AdbcDriverManager.cs
b/csharp/src/Apache.Arrow.Adbc/DriverManager/AdbcDriverManager.cs
index 3ebfc3bfc8..8eb2df3be8 100644
--- a/csharp/src/Apache.Arrow.Adbc/DriverManager/AdbcDriverManager.cs
+++ b/csharp/src/Apache.Arrow.Adbc/DriverManager/AdbcDriverManager.cs
@@ -134,12 +134,42 @@ namespace Apache.Arrow.Adbc.DriverManager
typeName: null,
manifestPath: null,
loadMethod: loadMethod,
- () => entrypoint == null
- ? CAdbcDriverImporter.LoadWithFallback(
- driverPath,
- resolvedEntrypoint,
- DefaultNativeEntrypoint)
- : CAdbcDriverImporter.Load(driverPath,
resolvedEntrypoint));
+ () => ImportNativeDriver(
+ driverPath,
+ () => entrypoint == null
+ ? CAdbcDriverImporter.LoadWithFallback(
+ driverPath,
+ resolvedEntrypoint,
+ DefaultNativeEntrypoint)
+ : CAdbcDriverImporter.Load(driverPath,
resolvedEntrypoint)));
+ }
+
+ /// <summary>
+ /// Runs a native driver import, reporting a library that is missing or
+ /// cannot be loaded as <see cref="AdbcStatusCode.NotFound"/> to match
the
+ /// C/C++ and Rust driver managers. The underlying load error is kept
as
+ /// the inner exception.
+ /// </summary>
+ private static AdbcDriver ImportNativeDriver(string driverPath,
Func<AdbcDriver> import)
+ {
+ if (!File.Exists(driverPath))
+ {
+ throw new AdbcException(
+ $"Driver library file not found: '{driverPath}'.",
+ AdbcStatusCode.NotFound);
+ }
+
+ try
+ {
+ return import();
+ }
+ catch (Exception ex) when (ex is DllNotFoundException || ex is
BadImageFormatException)
+ {
+ throw new AdbcException(
+ $"Could not load driver library '{driverPath}':
{ex.Message}",
+ AdbcStatusCode.NotFound,
+ ex);
+ }
}
//
-----------------------------------------------------------------------
@@ -853,11 +883,13 @@ namespace Apache.Arrow.Adbc.DriverManager
private static AdbcDriver LoadFromManifest(string manifestPath,
string? entrypoint)
{
+ // A manifest that can't be read is an invalid argument, as in the
+ // C/C++ and Rust driver managers
if (!File.Exists(manifestPath))
{
throw new AdbcException(
- $"Driver manifest file not found.",
- AdbcStatusCode.NotFound);
+ $"Driver manifest file not found: '{manifestPath}'.",
+ AdbcStatusCode.InvalidArgument);
}
DriverManifest manifest =
DriverManifest.LoadFromFile(manifestPath);
@@ -948,12 +980,14 @@ namespace Apache.Arrow.Adbc.DriverManager
typeName: null,
manifestPath: manifestPath,
loadMethod: loadMethod,
- () => allowNativeFallback
- ? CAdbcDriverImporter.LoadWithFallback(
- driverPath,
- entrypoint,
- DefaultNativeEntrypoint)
- : CAdbcDriverImporter.Load(driverPath, entrypoint));
+ () => ImportNativeDriver(
+ driverPath,
+ () => allowNativeFallback
+ ? CAdbcDriverImporter.LoadWithFallback(
+ driverPath,
+ entrypoint,
+ DefaultNativeEntrypoint)
+ : CAdbcDriverImporter.Load(driverPath, entrypoint)));
}
/// <summary>
@@ -997,8 +1031,9 @@ namespace Apache.Arrow.Adbc.DriverManager
// 2. Try native library extensions. If a matching file exists but
fails
// to load (e.g., corrupt binary, wrong architecture, missing
entrypoint),
- // surface that concrete error rather than silently degrading to a
generic
- // NotFound -- the file is clearly the user's intended driver.
+ // stop here rather than trying other directories -- the file is
clearly
+ // the user's intended driver. A library that cannot be loaded is
reported
+ // as NotFound with the concrete error attached (see
ImportNativeDriver).
foreach (string nativeExt in GetPlatformExtensions())
{
string libPath = Path.Combine(dir, driverName + nativeExt);
diff --git
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverNotFoundTests.cs
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverNotFoundTests.cs
new file mode 100644
index 0000000000..f57a8a047a
--- /dev/null
+++ b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverNotFoundTests.cs
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Runtime.InteropServices;
+using Apache.Arrow.Adbc.DriverManager;
+using Xunit;
+
+namespace Apache.Arrow.Adbc.Tests.DriverManager
+{
+ /// <summary>
+ /// A driver that does not exist should be reported as an
+ /// <see cref="AdbcException"/> with <see cref="AdbcStatusCode.NotFound"/>
+ /// no matter how it was specified or which search locations are enabled.
+ /// A library file that exists but cannot be loaded is also reported as
+ /// <see cref="AdbcStatusCode.NotFound"/>, with the load error attached,
+ /// matching the C/C++ driver manager. A manifest file that doesn't exist
+ /// is reported as <see cref="AdbcStatusCode.InvalidArgument"/>.
+ /// </summary>
+ [Collection(DriverManagerSecurityCollection.Name)]
+ public class DriverNotFoundTests : IDisposable
+ {
+ private const string MissingDriverName =
"adbc_test_driver_manager_missing_driver_7c8f5482";
+
+ private readonly List<string> _tempDirs = new List<string>();
+
+ public void Dispose()
+ {
+ foreach (string d in _tempDirs)
+ {
+ try { if (Directory.Exists(d)) Directory.Delete(d, true); }
catch { }
+ }
+ }
+
+ public static IEnumerable<object[]> AllLoadFlags()
+ {
+ AdbcLoadFlags all = AdbcLoadFlags.SearchEnv |
AdbcLoadFlags.SearchUser |
+ AdbcLoadFlags.SearchSystem | AdbcLoadFlags.AllowRelativePaths;
+ for (int flags = 0; flags <= (int)all; flags++)
+ {
+ yield return new object[] { (AdbcLoadFlags)flags };
+ }
+ }
+
+ [Theory]
+ [MemberData(nameof(AllLoadFlags))]
+ public void BareName_AllLoadFlags_ThrowsNotFound(AdbcLoadFlags flags)
+ {
+ AssertNotFound(MissingDriverName, flags);
+ }
+
+ [Theory]
+ [InlineData(AdbcLoadFlags.Default)]
+ [InlineData(AdbcLoadFlags.None)]
+ public void BareName_AdditionalSearchPath_ThrowsNotFound(AdbcLoadFlags
flags)
+ {
+ AssertNotFound(MissingDriverName, flags, MissingDriverDir());
+ }
+
+ [Theory]
+ [InlineData(AdbcLoadFlags.Default)]
+ [InlineData(AdbcLoadFlags.None)]
+ public void AbsolutePathWithoutExtension_ThrowsNotFound(AdbcLoadFlags
flags)
+ {
+ AssertNotFound(Path.Combine(MissingDriverDir(),
MissingDriverName), flags);
+ }
+
+ [Theory]
+ [InlineData(AdbcLoadFlags.Default)]
+ [InlineData(AdbcLoadFlags.None)]
+ public void
AbsolutePathWithLibraryExtension_ThrowsNotFound(AdbcLoadFlags flags)
+ {
+ string path = Path.Combine(MissingDriverDir(),
PlatformLibraryName(MissingDriverName));
+ AdbcException ex = AssertNotFound(path, flags);
+ Assert.Contains(path, ex.Message);
+ }
+
+ [Fact]
+ public void RelativePathWithLibraryExtension_ThrowsNotFound()
+ {
+ AssertNotFound(PlatformLibraryName(MissingDriverName),
AdbcLoadFlags.Compatible);
+ }
+
+ // A manifest file that doesn't exist is InvalidArgument rather than
+ // NotFound, as in the C/C++ and Rust driver managers
+ [Theory]
+ [InlineData(AdbcLoadFlags.Default)]
+ [InlineData(AdbcLoadFlags.None)]
+ public void AbsoluteManifestPath_ThrowsInvalidArgument(AdbcLoadFlags
flags)
+ {
+ string path = Path.Combine(MissingDriverDir(), MissingDriverName +
".toml");
+ AdbcException ex = Assert.Throws<AdbcException>(
+ () => AdbcDriverManager.FindLoadDriver(path, loadOptions:
flags));
+ Assert.Equal(AdbcStatusCode.InvalidArgument, ex.Status);
+ Assert.Contains(path, ex.Message);
+ }
+
+ [Fact]
+ public void RelativeManifestPath_ThrowsInvalidArgument()
+ {
+ string name = MissingDriverName + ".toml";
+ AdbcException ex = Assert.Throws<AdbcException>(
+ () => AdbcDriverManager.FindLoadDriver(name, loadOptions:
AdbcLoadFlags.Compatible));
+ Assert.Equal(AdbcStatusCode.InvalidArgument, ex.Status);
+ Assert.Contains(name, ex.Message);
+ }
+
+ [Fact]
+ public void ManifestWithMissingLibrary_ThrowsNotFound()
+ {
+ string dir = CreateTempDir();
+ string libraryPath = Path.Combine(dir,
PlatformLibraryName(MissingDriverName));
+ string manifestPath = WriteManifest(dir, libraryPath);
+
+ AdbcException ex = AssertNotFound(manifestPath,
AdbcLoadFlags.Default);
+ Assert.Contains(libraryPath, ex.Message);
+ }
+
+ //
-----------------------------------------------------------------------
+ // A library file that exists but is not a loadable shared library
+ //
-----------------------------------------------------------------------
+
+ [Fact]
+ public void InvalidLibrary_AbsolutePath_ThrowsNotFound()
+ {
+ string libraryPath = WriteInvalidLibrary(CreateTempDir(),
PlatformLibraryName(MissingDriverName));
+
+ AdbcException ex = AssertNotFound(libraryPath,
AdbcLoadFlags.Default);
+ Assert.Contains(libraryPath, ex.Message);
+ Assert.NotNull(ex.InnerException);
+ }
+
+ [Fact]
+ public void InvalidLibrary_BareNameInSearchPath_ThrowsNotFound()
+ {
+ string dir = CreateTempDir();
+ string extension =
RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".dll"
+ : RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? ".dylib"
+ : ".so";
+ string libraryPath = WriteInvalidLibrary(dir, MissingDriverName +
extension);
+
+ AdbcException ex = AssertNotFound(MissingDriverName,
AdbcLoadFlags.None, dir);
+ Assert.Contains(libraryPath, ex.Message);
+ Assert.NotNull(ex.InnerException);
+ }
+
+ [Fact]
+ public void ManifestWithInvalidLibrary_ThrowsNotFound()
+ {
+ string dir = CreateTempDir();
+ string libraryPath = WriteInvalidLibrary(dir,
PlatformLibraryName(MissingDriverName));
+ string manifestPath = WriteManifest(dir, libraryPath);
+
+ AdbcException ex = AssertNotFound(manifestPath,
AdbcLoadFlags.Default);
+ Assert.Contains(libraryPath, ex.Message);
+ Assert.NotNull(ex.InnerException);
+ }
+
+ private static AdbcException AssertNotFound(string driverName,
AdbcLoadFlags flags, string? additionalSearchPathList = null)
+ {
+ Exception? ex = Record.Exception(
+ () => AdbcDriverManager.FindLoadDriver(
+ driverName,
+ loadOptions: flags,
+ additionalSearchPathList: additionalSearchPathList));
+
+ Assert.True(
+ ex is AdbcException,
+ $"Expected AdbcException(NotFound), got
{ex?.GetType().FullName ?? "no exception"}: {ex?.Message}");
+ AdbcException adbcEx = (AdbcException)ex!;
+ Assert.Equal(AdbcStatusCode.NotFound, adbcEx.Status);
+ return adbcEx;
+ }
+
+ private string CreateTempDir()
+ {
+ string dir = Path.Combine(Path.GetTempPath(),
"adbc_driver_not_found_" + Guid.NewGuid().ToString("N"));
+ Directory.CreateDirectory(dir);
+ _tempDirs.Add(dir);
+ return dir;
+ }
+
+ private static string WriteInvalidLibrary(string dir, string fileName)
+ {
+ string path = Path.Combine(dir, fileName);
+ File.WriteAllText(path, "this is not a shared library");
+ return path;
+ }
+
+ private static string WriteManifest(string dir, string libraryPath)
+ {
+ string path = Path.Combine(dir, MissingDriverName + ".toml");
+ File.WriteAllText(path,
+ "manifest_version = 1\n" +
+ "name = \"Missing\"\n" +
+ "\n[Driver]\n" +
+ "shared = '" + libraryPath + "'\n");
+ return path;
+ }
+
+ private static string MissingDriverDir() =>
+ Path.Combine(Path.GetTempPath(),
"adbc_missing_driver_dir_7c8f5482");
+
+ private static string PlatformLibraryName(string name)
+ {
+ if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+ return name + ".dll";
+ if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+ return "lib" + name + ".dylib";
+ return "lib" + name + ".so";
+ }
+ }
+}
diff --git
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/TomlConnectionProfileTests.cs
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/TomlConnectionProfileTests.cs
index 540582d8c6..1538cde73b 100644
---
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/TomlConnectionProfileTests.cs
+++
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/TomlConnectionProfileTests.cs
@@ -667,7 +667,7 @@ oops = ""{{ env_var() }}""
string fakePath = Path.Combine(Path.GetTempPath(),
"fake_manifest_notexist.toml");
AdbcException ex = Assert.Throws<AdbcException>(
() => AdbcDriverManager.FindLoadDriver(fakePath));
- Assert.Equal(AdbcStatusCode.NotFound, ex.Status);
+ Assert.Equal(AdbcStatusCode.InvalidArgument, ex.Status);
}
[Fact]
