fix: try read relative uri's as files. #21
Conversation
Signed-off-by: Alex Wichmann <VisualBean@users.noreply.github.com>
WalkthroughDefaultStreamLoader now checks Uri.IsAbsoluteUri and normalizes absolute-uri schemes to lowercase. Absolute URIs branch on scheme: "file" -> open LocalPath, "http"/"https" -> fetch via HttpClient, others throw. Non-absolute URIs open the path via uri.OriginalString. Logic mirrored for Load and LoadAsync. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Loader as DefaultStreamLoader
participant FS as FileSystem
participant HTTP as HttpClient
Caller->>Loader: Load(uri) / LoadAsync(uri)
Loader->>Loader: if uri.IsAbsoluteUri?
alt Absolute URI
Loader->>Loader: scheme = uri.Scheme.ToLowerInvariant()
alt scheme == "file"
Loader->>FS: File.OpenRead(uri.LocalPath)
FS-->>Loader: Stream
Loader-->>Caller: Stream / Task<Stream>
else scheme == "http" or "https"
Loader->>HTTP: GetStreamAsync(uri)
HTTP-->>Loader: Stream
Loader-->>Caller: Task<Stream>
else other scheme
Loader-->>Caller: throw NotSupportedException
end
else Relative URI
Loader->>FS: File.OpenRead(uri.OriginalString)
FS-->>Loader: Stream
Loader-->>Caller: Stream / Task<Stream>
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Alex Wichmann <VisualBean@users.noreply.github.com>
Signed-off-by: Alex Wichmann <VisualBean@users.noreply.github.com>
Signed-off-by: Alex Wichmann <VisualBean@users.noreply.github.com>
Signed-off-by: Alex Wichmann <VisualBean@users.noreply.github.com>
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Signed-off-by: Alex Wichmann <VisualBean@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/ByteBard.AsyncAPI.Readers/Services/DefaultStreamLoader.cs (1)
31-34: Relative URIs: resolve against base document and strip fragment/query.Current code opens relative paths from the process working directory and includes any $ref fragment (e.g., "./file.yml#/components/..."), leading to false "file not found" errors. Relative references should resolve against the parent document’s location, and the loader must ignore URI fragment/query when opening the file. This is the core of issue #20 and remains unfixed in the relative branch. Based on relevant code snippet AsyncApiWorkspace.ToLocationUrl(string) returns UriKind.RelativeOrAbsolute, so relative inputs are expected here.
Minimum safe fix right now: strip fragment/query before File.OpenRead in both places.
- return File.OpenRead(uri.OriginalString); + var path = uri.OriginalString; + var hash = path.IndexOf('#'); + if (hash >= 0) path = path.Substring(0, hash); + var qm = path.IndexOf('?'); + if (qm >= 0) path = path.Substring(0, qm); + return File.OpenRead(path);However, to fully resolve #20 across contexts, pass a base URI (the current document location) into the loader and resolve via new Uri(baseUri, uri) before branching:
- Add an overload or extend IStreamLoader to accept a baseUri.
- Resolve with: var resolved = uri.IsAbsoluteUri ? uri : new Uri(baseUri, uri);
- Then reuse the absolute-URI switch.
I can draft a small refactor across IStreamLoader and call sites if you want.
Also applies to: 59-62
🧹 Nitpick comments (3)
src/ByteBard.AsyncAPI.Readers/Services/DefaultStreamLoader.cs (3)
18-29: Good guard and file-path fix; one follow-up for sync HTTP.
- IsAbsoluteUri check and using uri.LocalPath are correct. Nice.
- Consider the risk of blocking on HttpClient in Load(...) for http/https; this can deadlock under certain synchronization contexts and degrades throughput. Prefer steering callers to LoadAsync for network I/O or encapsulate the sync path behind a documented "may block" warning. Optionally, include the scheme in the "Unsupported scheme" error for easier diagnostics.
Example small tweak to improve the error text:
- throw new ArgumentException("Unsupported scheme"); + throw new ArgumentException($"Unsupported scheme '{uri.Scheme}'.")
46-58: Async path looks correct; consider CT and async-friendly file stream.
- Switched to await HttpClient.GetStreamAsync — good.
- Consider adding CancellationToken to LoadAsync and threading it through HTTP and file open.
- Optionally, for very large files or remote FS, return a FileStream opened with FileOptions.Asynchronous to enable true async reads downstream.
Example (signature change shown for illustration):
public async Task<Stream> LoadAsync(Uri uri, CancellationToken ct = default) { if (uri.IsAbsoluteUri) { switch (uri.Scheme.ToLowerInvariant()) { case "file": return new FileStream(uri.LocalPath, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, FileOptions.Asynchronous); case "http": case "https": return await HttpClient.GetStreamAsync(uri, ct); } } // relative path handling per the other comment (strip fragment/query and resolve with baseUri if available) }
38-38: Make error details more actionable.Include the resolved path/URI and inner exception type; helps users differentiate path resolution vs network errors.
- throw new AsyncApiReaderException($"Something went wrong trying to fetch '{uri.OriginalString}'. {ex.Message}", ex); + throw new AsyncApiReaderException( + $"Failed to load resource '{uri}'. {ex.GetType().Name}: {ex.Message}", + ex);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/ByteBard.AsyncAPI.Readers/Services/DefaultStreamLoader.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/ByteBard.AsyncAPI.Readers/Services/DefaultStreamLoader.cs (1)
src/ByteBard.AsyncAPI/AsyncApiWorkspace.cs (1)
Uri(246-249)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (csharp)
- GitHub Check: build (windows-latest)
About the PR
There is a bug, where the Uri is relative and fails
.Schemethrows and leaves us in the catch.Related Issues
Summary by CodeRabbit
Bug Fixes
Refactor