Skip to content

Commit 5b49a99

Browse files
committed
Ensure native exceptions are clear and properly propagated
Audit + fixes across the whole native<->managed boundary: - Grammar loading actually honors the native resolver now. NativeLibrary .Load(name, assembly, ...) does NOT invoke a SetDllImportResolver callback (that fires only for DllImport/LibraryImport), so grammar loads silently ignored TREE_SITTER_NATIVE_PATH and the repo native/<rid> fallback. Added internal NativeLibraryResolver.TryResolve(...) running the full probe sequence; LanguagePack.Load uses it (then the default loader), and InternalsVisibleTo exposes it. The documented contract is now true. - Manifest parse errors surface as InvalidOperationException with the JsonException chained, instead of a bare JsonException; added a string/Stream test seam. Verified the rest of the boundary already surfaces clear, typed, chained exceptions (lib/export/null-ptr load failures, allocation failures, ABI mismatch, query compile errors, no-language vs cancel, dispose, the single intentional logger-thunk boundary swallow, no throw-ex stack resets, buffers freed on throw paths). Adds tests/ErrorPropagationTests.cs (23 cases) proving each path. Build 0/0 (Debug+Release); 404 tests pass (incl. hermetic); coverage 97.4% / 98.0% / 99.5% (core / codegen / languagepack). https://claude.ai/code/session_01S3hwvd2oz6cntXmB3G6Yf1
1 parent 3fc1d5e commit 5b49a99

5 files changed

Lines changed: 567 additions & 8 deletions

File tree

src/TreeSitter.LanguagePack/LanguagePack.cs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,21 @@ private static unsafe Language Load(LanguageInfo info)
187187
nint handle;
188188
try
189189
{
190-
// Load via a PUBLIC TreeSitter type so the NativeLibraryResolver — which is
191-
// registered on the TreeSitter assembly only — fires for this grammar load.
192-
// This makes TREE_SITTER_NATIVE_PATH and the repo native/<rid>/ dev fallback
193-
// apply to grammars too (see docs/ARCHITECTURE.md and LanguageNotAvailableException).
194-
handle = System.Runtime.InteropServices.NativeLibrary.Load(
195-
info.NativeLibraryName, typeof(global::TreeSitter.Language).Assembly, null);
190+
// Resolve through the TreeSitter NativeLibraryResolver's probe sequence so
191+
// TREE_SITTER_NATIVE_PATH, the runtimes/<rid>/native layout, the assembly
192+
// directory, and the repo native/<rid>/ dev fallback ALL apply to grammar
193+
// loads (see docs/ARCHITECTURE.md and LanguageNotAvailableException).
194+
//
195+
// We must call the resolver explicitly: NativeLibrary.Load(name, assembly,
196+
// searchPath) does NOT invoke the DllImport resolver registered via
197+
// SetDllImportResolver — that callback only fires for DllImport/LibraryImport
198+
// P/Invoke. So delegate to the resolver's probe first, then fall back to the
199+
// default loader (which honours the OS search path and any embedded rpath).
200+
if (!global::TreeSitter.Native.NativeLibraryResolver.TryResolve(info.NativeLibraryName, out handle))
201+
{
202+
handle = System.Runtime.InteropServices.NativeLibrary.Load(
203+
info.NativeLibraryName, typeof(global::TreeSitter.Language).Assembly, null);
204+
}
196205
}
197206
catch (Exception ex) when (ex is DllNotFoundException or System.IO.FileNotFoundException or BadImageFormatException)
198207
{

src/TreeSitter.LanguagePack/Manifests.cs

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,50 @@ internal static (FrozenDictionary<string, LanguageInfo> Manifest,
5353
private static Dictionary<string, ManifestEntry> DeserializeRaw()
5454
{
5555
using Stream stream = OpenManifest();
56-
Dictionary<string, ManifestEntry>? raw =
57-
JsonSerializer.Deserialize(stream, ManifestJsonContext.Default.DictionaryStringManifestEntry);
56+
return DeserializeRaw(stream);
57+
}
58+
59+
/// <summary>
60+
/// Parses manifest JSON from an arbitrary <paramref name="stream"/> into its raw
61+
/// entries, throwing <see cref="InvalidOperationException"/> with a clear message
62+
/// when the document is empty, <c>null</c>, or contains no entries. This is the
63+
/// single validation seam shared by the embedded-resource read and the tests; it
64+
/// keeps the public <see cref="LoadAll"/> behavior identical while letting the
65+
/// empty/corrupt-manifest path be exercised with synthetic input.
66+
/// </summary>
67+
/// <param name="stream">A UTF-8 JSON stream holding a <c>name -&gt; entry</c> object.</param>
68+
internal static Dictionary<string, ManifestEntry> DeserializeRaw(Stream stream)
69+
{
70+
Dictionary<string, ManifestEntry>? raw;
71+
try
72+
{
73+
raw = JsonSerializer.Deserialize(stream, ManifestJsonContext.Default.DictionaryStringManifestEntry);
74+
}
75+
catch (JsonException ex)
76+
{
77+
// A malformed manifest must surface as a clear, typed error with the parse
78+
// failure chained — never a bare JsonException leaking out of the loader.
79+
throw new InvalidOperationException(
80+
"The embedded language manifest is empty or could not be parsed.", ex);
81+
}
5882

5983
if (raw is null || raw.Count == 0)
6084
throw new InvalidOperationException("The embedded language manifest is empty or could not be parsed.");
6185

6286
return raw;
6387
}
6488

89+
/// <summary>
90+
/// Parses manifest JSON from a <paramref name="json"/> string. A convenience wrapper
91+
/// over <see cref="DeserializeRaw(Stream)"/> for tests; shares the same validation.
92+
/// </summary>
93+
/// <param name="json">The manifest JSON text.</param>
94+
internal static Dictionary<string, ManifestEntry> DeserializeRaw(string json)
95+
{
96+
using var stream = new MemoryStream(System.Text.Encoding.UTF8.GetBytes(json));
97+
return DeserializeRaw(stream);
98+
}
99+
65100
/// <summary>
66101
/// Pure index builder, separated from the embedded-resource read so it can be unit
67102
/// tested with synthetic manifests and ambiguity maps. Each extension (lowercase, no

src/TreeSitter/Native/NativeLibraryResolver.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,33 @@ internal static IntPtr Resolve(string libraryName, Assembly assembly, DllImportS
9999
return handle;
100100
}
101101

102+
/// <summary>
103+
/// Runs the binding's native-library probing sequence for <paramref name="logicalName"/>
104+
/// (the same well-known locations the <see cref="DllImportResolver"/> uses: the
105+
/// <c>TREE_SITTER_NATIVE_PATH</c> override, the NuGet <c>runtimes/&lt;rid&gt;/native</c>
106+
/// layout, the assembly directory, and the repo <c>native/&lt;rid&gt;/</c> dev
107+
/// fallback), returning the loaded handle on success.
108+
/// </summary>
109+
/// <remarks>
110+
/// This is the explicit entry point grammar loaders (notably
111+
/// <c>TreeSitter.LanguagePack</c>) must call, because
112+
/// <see cref="NativeLibrary.Load(string, Assembly, DllImportSearchPath?)"/> does
113+
/// <b>not</b> invoke a resolver registered via
114+
/// <see cref="NativeLibrary.SetDllImportResolver"/> — that callback is wired only
115+
/// into the <c>DllImport</c>/<c>LibraryImport</c> path. Routing grammar loads
116+
/// through here keeps <c>TREE_SITTER_NATIVE_PATH</c> and the dev fallback working
117+
/// for grammars exactly as they do for the core engine. Successful resolutions are
118+
/// cached by logical name, mirroring <see cref="Resolve"/>.
119+
/// </remarks>
120+
/// <param name="logicalName">The logical library name (e.g. <c>tree-sitter-python</c>).</param>
121+
/// <param name="handle">On success, the loaded native handle; otherwise <see cref="IntPtr.Zero"/>.</param>
122+
/// <returns><see langword="true"/> if the library was resolved and loaded.</returns>
123+
internal static bool TryResolve(string logicalName, out IntPtr handle)
124+
{
125+
handle = Resolve(logicalName, typeof(NativeLibraryResolver).Assembly, searchPath: null);
126+
return handle != IntPtr.Zero;
127+
}
128+
102129
/// <summary>Runs the full probing sequence for an unresolved logical name.</summary>
103130
private static IntPtr ResolveCore(string libraryName, Assembly assembly)
104131
{

src/TreeSitter/TreeSitter.csproj

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@
3636
public API is unchanged. -->
3737
<ItemGroup>
3838
<InternalsVisibleTo Include="TreeSitter.Tests" />
39+
<!-- LanguagePack resolves grammar libraries through this assembly's
40+
NativeLibraryResolver (so TREE_SITTER_NATIVE_PATH and the repo native/<rid>/
41+
dev fallback apply to grammar loads). NativeLibrary.Load does NOT invoke the
42+
DllImport resolver, so the pack must call the resolver's probe directly. -->
43+
<InternalsVisibleTo Include="TreeSitter.LanguagePack" />
3944
</ItemGroup>
4045

4146
<!-- NuGet package metadata. -->

0 commit comments

Comments
 (0)