Throw ArgumentException for a malformed baseUrl, as documented

[dmccoystephenson]

Summary

• The constructor's XML doc says it throws ArgumentException for a "missing or malformed baseUrl". A missing one did, but a malformed one got out of new Uri(...) as UriFormatException, which is a FormatException and not an ArgumentException. A caller catching the documented type missed it.
• The new Uri(...) call is now wrapped. UriFormatException is rethrown as ArgumentException("baseUrl is not a valid URL", "baseUrl", inner), which matches the sibling precondition messages and keeps the original as InnerException.
• The code now does what the docs already said. The documented contract is unchanged, so no README or XML doc edits are needed.
• New test: Constructor_RejectsAMalformedBaseUrlWithAnArgumentException. It checks the exact type (Assert.Throws<ArgumentException> does not accept subtypes), ParamName and the inner UriFormatException.

Closes #3

Test plan

• Build workflow green on ubuntu-latest (net8.0) and windows-latest (net8.0 + net48)
• Regression evidence: the fix is reverted on CI while the new test is kept, the test is confirmed red, then the fix is restored (no local dotnet SDK was available in the drafting sandbox)

Notes for the reviewer

• Whether the constructor may throw UriFormatException or only ArgumentException is arguably part of the public contract. This change makes the code follow the documented contract instead of changing that contract. It still needs a maintainer's review before merge.
• Unity (Mono/IL2CPP) is not exercised by CI. The change only touches constructor-time URI parsing, which is not expected to behave differently there.

Skipped issues: none. There were no other open issues at triage time.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

🤖 Generated with Claude Code

[dmccoystephenson]

Self-review rubric (head a662e9b):

• Scope: PASS. Two files: the constructor's new Uri(...) call and one new test. No unrelated formatting or renames.
• Tests-new: PASS (not applicable). No public member was added. The changed constructor behaviour is covered by Constructor_RejectsAMalformedBaseUrlWithAnArgumentException.
• Tests-fix: PASS, shown on CI because no local dotnet SDK was available. Commit 45c6b6c (TEMP: revert a662e9b ...) reverted only TraceClient.cs. Run 36684222938 went red on exactly the new test (Expected: typeof(System.ArgumentException) / Actual: typeof(System.UriFormatException), Failed: 1, Passed: 50, Total: 51 on net8.0). The branch was then reset and force-pushed back to a662e9b, and run 36684397257 is green on both legs.
• Sibling structure: PASS. The message "baseUrl is not a valid URL" and paramName "baseUrl" follow the neighbouring "baseUrl is required" precondition.
• Sibling renames: PASS (not applicable). Nothing was renamed.
• Docs: PASS. The constructor XML doc already promised ArgumentException for a malformed baseUrl, and the code now matches it. The README says nothing about constructor exceptions, and the header, class <remarks> and README tables are unaffected.
• Issue resolution: PASS. Constructor throws UriFormatException, not the documented ArgumentException, for a malformed baseUrl #3 names the new Uri(...) call at TraceClient.cs:140, which is the line changed.
• CI: PASS. Run 36684080564 (and the re-run 36684397257) on a662e9b: Passed: 51, Total: 51 on ubuntu net8.0, windows net8.0 and windows net48, up from 50 on main.
• No dependencies / Single file / Vendoring floor: PASS. The csproj is untouched, the using list is unchanged, src/TraceClient still holds only TraceClient.cs and TraceClient.csproj, and the net48 leg is green.
• Never throws: PASS. The only new throw is in the constructor's precondition section, where throwing is documented. Nothing reachable from Report/Close/Dispose/Drain/Send/Log changed.
• Non-blocking & bounded / Environment seam / Version agreement / Opt-out contract: PASS (not applicable). None of those paths or strings changed.

Judgment calls and out-of-diff observations:

• src/TraceClient/TraceClient.cs:140 — Which exception type the public constructor throws is arguably part of the public-API contract (do-not-auto-merge list). The change makes the code follow the already-documented contract rather than altering it, but it still needs a maintainer's review.
• src/TraceClient/TraceClient.cs:142 — Out of scope and not changed here: some strings parse without a UriFormatException but still are not usable HTTP endpoints. For example, a baseUrl starting with / becomes a file:// URI on .NET Core/Unix, and ftp://host is accepted. Checking for absolute http/https would be a contract change affecting parity with the Java and Python clients, so it was left alone.
• CI does not exercise Unity (Mono/IL2CPP). The change only affects constructor-time parsing and uses UriFormatException, which exists on every target, so no platform difference is expected.

This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson