Throw argument exceptions synchronously in Core's Task extensions

.claude/agents/reviewer.md

@@ -15,7 +15,7 @@ First read `CLAUDE.md` and every **accepted** ADR in `docs/adr/` that relates to
 ## Checklist
 
 **ADR compliance**
-- ADR-0002: no `catch (Exception)` or bare `catch`. Expected failures (HTTP status, payload, network, timeout) are returned as `OpenUrzednikResult` errors. Only caller cancellation, argument errors and fatal errors are thrown; in provider client methods, a `null` argument or an undefined enum value throws synchronously, before the task is returned (flag an `async` public client method that checks them, or a `ValidationError` for them), while a non-null value of the right type that fails a format or range check is a returned `ValidationError`. Timeouts are distinguished from caller cancellation. Errors implement `CreateException()` returning `OpenUrzednikException` subtypes; exceptions keep their error in `Error`/`Errors`. Several validation errors are thrown as one `OpenUrzednikValidationException` listing all of them, never `AggregateException`; several errors of other kinds as the first error's exception with all of them in `Errors`. Resilience-handler rejections are translated in DI packages, not caught in providers.
+- ADR-0002: no `catch (Exception)` or bare `catch`. Expected failures (HTTP status, payload, network, timeout) are returned as `OpenUrzednikResult` errors. Only caller cancellation, argument errors and fatal errors are thrown; in every public method of a public type that returns a `Task` (provider clients, Core's extensions on `Task<…>`), a `null` argument or an undefined enum value throws synchronously, before the task is returned (flag a public `async` method that checks them, or a `ValidationError` for them), while a non-null value of the right type that fails a format or range check is a returned `ValidationError`. Timeouts are distinguished from caller cancellation. Errors implement `CreateException()` returning `OpenUrzednikException` subtypes; exceptions keep their error in `Error`/`Errors`. Several validation errors are thrown as one `OpenUrzednikValidationException` listing all of them, never `AggregateException`; several errors of other kinds as the first error's exception with all of them in `Errors`. Resilience-handler rejections are translated in DI packages, not caught in providers.
 - ADR-0003: no `ILogger`/`ActivitySource` in Core or provider packages, except Core's internal `Telemetry/Activities` (the .NET fallback trace source, compiled on .NET only). Members of those types that other packages call through `InternalsVisibleTo` are a binary contract: flag a change or removal outside a major. Span and tag naming; HTTP tags use the OpenTelemetry names (`http.request.method`, `url.path`, `http.response.status_code`); errors set `error.type`. Activity sources are named `OpenUrzednik.<Provider>`. Every log call passes an event id; a new log event has a new id in `RestLogEvents` (HTTP, 1xxx) or the provider's `<Provider>LogEvents` (2xxx); flag a renumbered, reused or missing id, and a `RestLogEvents` entry not mirrored in `<Provider>LogEvents`. Flag any change to `IOpenUrzednikLogger`, `IOpenUrzednikTraceSource` or `IOpenUrzednikSpan` after 1.0 (frozen; new capabilities are separate interfaces), and any change to a stable name (source, span, documented tag, `error.type`, logger category, event id) outside a major. `IsEnabled` guards. No secrets (API keys, session ids, tokens) or personal identifiers (PESEL, NIP, REGON, bank account numbers) in logs, span names or tags, error and exception messages, error metadata or `Exception.Data`; at most the last 4 characters.
 - ADR-0004: no new `PackageReference` in Core or provider projects, apart from netstandard2.0-only official Microsoft BCL packages (currently `System.Text.Json`, `System.Net.Http.Json`, `Microsoft.Bcl.TimeProvider`, `Microsoft.Bcl.HashCode`; any other one needs approval). `System.Diagnostics.DiagnosticSource` is allowed only in `OpenUrzednik.Diagnostics`. No `Version=` on package references (central package management). A dependency of a published library moves only by patch unless the PR says why a minor or major raise is needed; the DI package's `Microsoft.Extensions.*` keep the `net8.0` override (8.0.x), and no package gets two conditional `Include` rows. DI packages don't add a resilience handler themselves (the app chains `AddStandardResilienceHandler()`), and they translate its rejection exceptions into errors.
 - ADR-0005: code builds for netstandard2.0 and passes `test (net472)`. Package validation stays on with no suppressions: flag a `CompatibilitySuppressions.xml`, a `CPxxxx` in `NoWarn`, or `EnablePackageValidation` turned off (the only exception is ADR-0011's CP0006 for a new client-interface method). `OpenUrzednik.snk`, `SignAssembly`, `PublicKey` and `assemblyVersion.precision` don't change; a project doesn't turn signing off; a third-party `InternalsVisibleTo` friend has its own `Key`. `#if` kept inside helpers (polyfills in `src/Polyfills`, `src/Http/Polyfills`, `Nbp/Polyfills`, `Extensions.Logging/Polyfills`). Every build has the same public signatures for what it shares: flag a public `DateOnly` (parameter, property or return type) and any interface member that exists only on .NET; public dates are `DateTime`, with `DateOnly` overloads only as .NET-only extension methods. Internally, dates are written as `DateOnly` (an alias for `DateTime` on netstandard2.0) and compared by `DayNumber`. A .NET target that isn't the lowest one may be removed in a minor version once Microsoft ends its support; the lowest .NET target and netstandard2.0 are removed only in a major. Targets are netstandard2.0 plus the .NET LTS releases in support.

CLAUDE.md

@@ -40,7 +40,7 @@ Before you say a task is done, run `dotnet build`, `dotnet test -f net10.0` and
 
 **Errors ([ADR-0002](docs/adr/0002-result-pattern-and-error-handling.md))**
 - Public async APIs return `Task<OpenUrzednikResult<T>>`. Failures that can happen during normal execution are returned, not thrown: validation, every non-success HTTP status, bad payloads, network errors, timeouts.
-- Throw only for caller cancellation (`OperationCanceledException` when the caller's token is cancelled), programmer errors (`ArgumentNullException`, invalid options) and fatal errors. In provider clients, a `null` argument and an undefined enum value (`ArgumentOutOfRangeException`) throw **synchronously**: the public method checks them and returns the task of a private `…CoreAsync` method. A non-null value of the right type that fails a format or range check is a returned `ValidationError`.
+- Throw only for caller cancellation (`OperationCanceledException` when the caller's token is cancelled), programmer errors (`ArgumentNullException`, invalid options) and fatal errors. In every public method of a public type that returns a `Task`, a `null` argument and an undefined enum value (`ArgumentOutOfRangeException`) throw **synchronously**: the method isn't `async` and checks them before it creates the task (typically it returns the task of a private `…CoreAsync` method). A non-null value of the right type that fails a format or range check is a returned `ValidationError`.
 - **Never write `catch (Exception)` or a bare `catch`.** Catch specific types and convert them to errors.
 - A timeout is an `OperationCanceledException` while `cancellationToken.IsCancellationRequested == false`. Return it as an error.
 - New error types derive from `OpenUrzednikError`, define `public const string ErrorCode`, and implement `CreateException()` returning an `OpenUrzednikException` subtype. The public `ToException()` calls it and attaches the error (`OpenUrzednikException.Error`/`Errors`).

CONTRIBUTING.md

@@ -15,7 +15,7 @@
 
 Binding architecture decisions are in [`docs/adr/`](docs/adr/README.md), and current work and priorities are in the [issues](https://github.com/milten89/OpenUrzednik.NET/issues) and the [project](https://github.com/users/milten89/projects/2). A change that contradicts an accepted ADR needs a new ADR in the same PR. The Code conventions, Testing conventions and Performance sections of [`CLAUDE.md`](CLAUDE.md) apply to every contributor; using AI tools is optional.
 
-- **`OpenUrzednikResult` / `OpenUrzednikResult<T>` for every error that can happen during normal operation** (validation, HTTP statuses, bad responses, network errors, timeouts), instead of error handling scattered across the code. We throw exceptions only when the caller cancels, for programmer errors (a `null` argument or an undefined enum value, which a client method throws before it returns its task) and for fatal errors; a value of the right type that fails a format or range check is a returned `ValidationError`. Don't use `catch (Exception)` ([ADR-0002](docs/adr/0002-result-pattern-and-error-handling.md)). Exceptions (`OpenUrzednikException` and its subtypes) are available through `.EnsureSuccess()` / `.EnsureSuccessAsync()` for consumers who prefer the classic style; don't add parallel API variants.
+- **`OpenUrzednikResult` / `OpenUrzednikResult<T>` for every error that can happen during normal operation** (validation, HTTP statuses, bad responses, network errors, timeouts), instead of error handling scattered across the code. We throw exceptions only when the caller cancels, for programmer errors (a `null` argument or an undefined enum value, which a public method that returns a task throws before it returns the task) and for fatal errors; a value of the right type that fails a format or range check is a returned `ValidationError`. Don't use `catch (Exception)` ([ADR-0002](docs/adr/0002-result-pattern-and-error-handling.md)). Exceptions (`OpenUrzednikException` and its subtypes) are available through `.EnsureSuccess()` / `.EnsureSuccessAsync()` for consumers who prefer the classic style; don't add parallel API variants.
 - **Dependencies:** the Core and provider packages have no NuGet dependencies (apart from official BCL packages for netstandard2.0); integrations with `Microsoft.Extensions.*` go into separate packages ([ADR-0004](docs/adr/0004-dependency-policy.md)).
 - **`OpenUrzednikError`** is the base type for expected errors. A new error is a `sealed` class ([ADR-0011](docs/adr/0011-compatibility-policy.md)) that derives from it ([ADR-0002](docs/adr/0002-result-pattern-and-error-handling.md)), defines `public const string ErrorCode`, passes it to the base constructor and overrides `CreateException()` to return an exception derived from `OpenUrzednikException` (see `NotFoundError`).
 - **Provider packages** follow the patterns in `OpenUrzednik.Core`: type names, error naming and the way results are returned stay consistent.

docs/adr/0002-result-pattern-and-error-handling.md

@@ -57,7 +57,7 @@ The metadata keys are public constants next to the errors that set them: `OpenUr
 **Thrown:**
 
 * Caller cancellation – `OperationCanceledException` when the caller's `CancellationToken` was cancelled.
-* Programmer errors – `ArgumentNullException`, `ArgumentException`, invalid configuration detected in constructors/options. In the provider clients' public methods, a `null` argument (`ArgumentNullException`) and a value that isn't a defined enum member (`ArgumentOutOfRangeException`) are thrown **synchronously**, before the method returns its task, as the task-based asynchronous pattern says for usage errors: the method checks them, then returns the task of a private `async` method.
+* Programmer errors – `ArgumentNullException`, `ArgumentException`, invalid configuration detected in constructors/options. In every public method of a public type that returns a task (the provider clients' methods, Core's extensions on `Task<…>`), a `null` argument (`ArgumentNullException`) and a value that isn't a defined enum member (`ArgumentOutOfRangeException`) are thrown **synchronously**, before the method returns its task, as the task-based asynchronous pattern says for usage errors: the method isn't `async` and checks them before it creates the task, typically by returning the task of a private `async` method.
 * Fatal runtime conditions – `OutOfMemoryException`, `StackOverflowException`, etc. are never caught.
 
 **Rules for code:**
@@ -100,7 +100,7 @@ Supersedes the informal rule in `CONTRIBUTING.md`. Shared HTTP behaviour is impl
 
 **2026-10-17 change: one list of error paths** ([#77](https://github.com/milten89/OpenUrzednik.NET/issues/77)). The Confirmation, `CLAUDE.md` and the `/new-endpoint` skill listed different error paths: `CLAUDE.md` left out 401/403, and the skill also left out the connection failure. The Confirmation now lists every path the WireMock tests already cover for each NBP client, including the 200 with an empty array for endpoints that return one, and the other two documents point to it.
 
-**2026-10-29 change: argument errors are thrown** ([#89](https://github.com/milten89/OpenUrzednik.NET/issues/89)). Before, the NBP clients returned a `ValidationError` for a `null` currency code and an undefined `TableType`, while the rules above and `CONTRIBUTING` named `ArgumentNullException` for a `null` argument, so callers couldn't tell from the docs whether to catch an exception or check the result. The maintainer drew the line where the .NET design guidelines do: a usage error (a `null` argument or an undefined enum value) throws, synchronously; a non-null value of the right type that fails a format or range check is a returned `ValidationError`. Core's extensions on `Task<…>` (`EnsureSuccessAsync`, `MapAsync`, `BindAsync`) still check their arguments inside an `async` method; that is a separate change. It affects the 15 methods that take a currency code or a `TableType`; the gold methods take neither. It is a breaking change for preview users.
+**2026-10-29 change: argument errors are thrown** ([#89](https://github.com/milten89/OpenUrzednik.NET/issues/89)). Before, the NBP clients returned a `ValidationError` for a `null` currency code and an undefined `TableType`, while the rules above and `CONTRIBUTING` named `ArgumentNullException` for a `null` argument, so callers couldn't tell from the docs whether to catch an exception or check the result. The maintainer drew the line where the .NET design guidelines do: a usage error (a `null` argument or an undefined enum value) throws, synchronously; a non-null value of the right type that fails a format or range check is a returned `ValidationError`. Core's extensions on `Task<…>` (`EnsureSuccessAsync`, `MapAsync`, `BindAsync`) still check their arguments inside an `async` method; that is a separate change (done on 2026-10-30, #159). It affects the 15 methods that take a currency code or a `TableType`; the gold methods take neither. It is a breaking change for preview users.
 
 **2026-10-29 change: the 1.0 error contract** ([#90](https://github.com/milten89/OpenUrzednik.NET/issues/90)). The "Returned" list became the mapping table above, and the compatibility rule, the reasons for departing from the design guidelines, the `default(OpenUrzednikResult)` rule and the `Value` rule were added. Changes made with it:
 
@@ -113,3 +113,5 @@ Supersedes the informal rule in `CONTRIBUTING.md`. Shared HTTP behaviour is impl
 These are breaking changes for preview users.
 
 **2026-10-30 change: masked values in validation errors** ([#109](https://github.com/milten89/OpenUrzednik.NET/issues/109)). `ValueValidator<T>.GetValidationErrorResult` stores the validated value under `ValueMetadataKey`, so a validator for a personal identifier would copy the identifier into every error, exception and log that shows the metadata. A validator for a personal identifier must pass a masked value, under the rule in [ADR-0003](0003-telemetry-abstractions.md). NBP validates no such value, so nothing changes there.
+
+**2026-10-30 change: synchronous argument errors in every task-returning method** ([#159](https://github.com/milten89/OpenUrzednik.NET/issues/159)). The rule from #89 covered only the provider clients. Core's extensions on `Task<…>` (`EnsureSuccessAsync`, `MapAsync`, `BindAsync`) checked their arguments inside an `async` method, so a `null` task or delegate came back as a faulted task. They now check their arguments, then return the task of a private `async` method, and the rule covers every public method of a public type that returns a task. It is a breaking change for callers who observed the exception on the task.