Skip to content

Commit 428e614

Browse files
committed
Preserve exact null parameter names in DI and hosting extensions
2 parents 3bc9907 + 6ec7add commit 428e614

9 files changed

Lines changed: 261 additions & 16 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
88
## [Unreleased]
99

1010
### Fixed
11+
- DI and Hosting registration extensions now preserve each null argument's public parameter name in `ArgumentNullException.ParamName`.
1112
- Fixed the `AddProcessKitGroup` configuration overload to report `services` as the null argument.
1213
- Runners registered by `AddProcessKit` and `AddProcessKitGroup` now reject a null command synchronously with `ArgumentNullException` naming `command` before applying defaults, logging, cancellation, or invoking the underlying runner.
1314
- `AddProcessKitClient(..., configure)` now rejects a null configured client with `ArgumentNullException` naming `configure` when the keyed client is resolved, instead of reporting a missing keyed registration.

‎README.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,6 +1079,10 @@ container has an `ILoggerFactory`). The runners registered by `AddProcessKit()`
10791079
(`ParamName = "command"`) before applying defaults or logging and before delegating to their
10801080
underlying runner. A keyed `AddProcessKitClient(..., configure)` callback must likewise return a
10811081
non-null `CliClient`; resolution rejects a null result with `ArgumentNullException` naming `configure`.
1082+
The DI registration overloads also name each null argument after their public signatures
1083+
(`services`, `configure`, `configuration`, `name`, or `program`); the Hosting registration and
1084+
configuration overloads do the same for `services`, `name`, `command`, `configureSupervisor`, and
1085+
`configure`.
10821086

10831087
*Deeper: [Observability](docs/observability.md).*
10841088

‎docs/dependency-injection.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ builder. The callback runs when the keyed client is resolved and must return a n
6161
null result is rejected with `ArgumentNullException` naming `configure` instead of being reported as a
6262
missing keyed registration.
6363

64+
Across the DI registration overloads, a null argument is rejected with `ArgumentNullException` whose
65+
`ParamName` matches the public signature: `services`, `configure`, `configuration`, `name`, or
66+
`program`. The configured-client result check above remains deferred until keyed-client resolution and
67+
names the callback parameter, `configure`.
68+
6469
```csharp
6570
services.AddProcessKit();
6671
services.AddProcessKitClient("git", "git", c => c.WithDefaults(cmd => cmd.CurrentDir("/repo")));
@@ -115,6 +120,11 @@ services.ConfigureProcessKitHostedProcess("worker", o =>
115120
});
116121
```
117122

123+
These Hosting extensions preserve the same diagnostic contract: a null argument reports its public
124+
parameter name (`services`, `name`, `command`, `configureSupervisor`, or `configure`) from
125+
`AddProcessKitHostedProcess`, `ConfigureProcessKitHostedProcess`, and
126+
`AddProcessKitHostedProcessHealthCheck`.
127+
118128
Resolve `HostedProcessService` by the same key when you need the last `SupervisionOutcome` or stop
119129
outcome for health reporting. It also exposes live supervision telemetry — `IsSupervisionActive`,
120130
`RestartCount`, `IsStormPaused` — for anything that wants to observe the child without waiting for

‎src/ProcessKit.Extensions.DependencyInjection/ServiceCollectionExtensions.fs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ type ServiceCollectionExtensions =
121121
(services: IServiceCollection, configure: Action<ProcessKitOptions>)
122122
: IServiceCollection =
123123
ArgumentNullException.ThrowIfNull(services, nameof services)
124-
ArgumentNullException.ThrowIfNull configure
124+
ArgumentNullException.ThrowIfNull(configure, nameof configure)
125125
services.Configure configure |> ignore
126126
ServiceCollectionExtensions.AddProcessKit services
127127

@@ -135,7 +135,7 @@ type ServiceCollectionExtensions =
135135
[<RequiresDynamicCode "Binds ProcessKitOptions from IConfiguration by reflection; use the Action<ProcessKitOptions> overload in a NativeAOT app.">]
136136
static member AddProcessKit(services: IServiceCollection, configuration: IConfiguration) : IServiceCollection =
137137
ArgumentNullException.ThrowIfNull(services, nameof services)
138-
ArgumentNullException.ThrowIfNull configuration
138+
ArgumentNullException.ThrowIfNull(configuration, nameof configuration)
139139
services.Configure<ProcessKitOptions> configuration |> ignore
140140
ServiceCollectionExtensions.AddProcessKit services
141141

@@ -161,9 +161,9 @@ type ServiceCollectionExtensions =
161161
(services: IServiceCollection, name: string, program: string, configure: Func<CliClient, CliClient>)
162162
: IServiceCollection =
163163
ArgumentNullException.ThrowIfNull(services, nameof services)
164-
ArgumentNullException.ThrowIfNull name
165-
ArgumentNullException.ThrowIfNull program
166-
ArgumentNullException.ThrowIfNull configure
164+
ArgumentNullException.ThrowIfNull(name, nameof name)
165+
ArgumentNullException.ThrowIfNull(program, nameof program)
166+
ArgumentNullException.ThrowIfNull(configure, nameof configure)
167167

168168
if DiInternals.hasClient services name then
169169
raise (
@@ -221,7 +221,7 @@ type ServiceCollectionExtensions =
221221
(services: IServiceCollection, configure: Action<ProcessKitOptions>)
222222
: IServiceCollection =
223223
ArgumentNullException.ThrowIfNull(services, nameof services)
224-
ArgumentNullException.ThrowIfNull configure
224+
ArgumentNullException.ThrowIfNull(configure, nameof configure)
225225
services.Configure configure |> ignore
226226
ServiceCollectionExtensions.AddProcessKitGroup services
227227

@@ -235,6 +235,6 @@ type ServiceCollectionExtensions =
235235
[<RequiresDynamicCode "Binds ProcessKitOptions from IConfiguration by reflection; use the Action<ProcessKitOptions> overload in a NativeAOT app.">]
236236
static member AddProcessKitGroup(services: IServiceCollection, configuration: IConfiguration) : IServiceCollection =
237237
ArgumentNullException.ThrowIfNull(services, nameof services)
238-
ArgumentNullException.ThrowIfNull configuration
238+
ArgumentNullException.ThrowIfNull(configuration, nameof configuration)
239239
services.Configure<ProcessKitOptions> configuration |> ignore
240240
ServiceCollectionExtensions.AddProcessKitGroup services

‎src/ProcessKit.Extensions.Hosting/HostedServiceCollectionExtensions.fs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ type HostedServiceCollectionExtensions =
4646
[<Extension>]
4747
static member AddProcessKitHostedProcess
4848
(services: IServiceCollection, name: string, command: Command, configureSupervisor: Func<Supervisor, Supervisor>) : IServiceCollection =
49-
ArgumentNullException.ThrowIfNull services
50-
ArgumentNullException.ThrowIfNull name
51-
ArgumentNullException.ThrowIfNull command
52-
ArgumentNullException.ThrowIfNull configureSupervisor
49+
ArgumentNullException.ThrowIfNull(services, nameof services)
50+
ArgumentNullException.ThrowIfNull(name, nameof name)
51+
ArgumentNullException.ThrowIfNull(command, nameof command)
52+
ArgumentNullException.ThrowIfNull(configureSupervisor, nameof configureSupervisor)
5353

5454
if Registration.hasHostedProcess services name then
5555
raise (
@@ -103,9 +103,9 @@ type HostedServiceCollectionExtensions =
103103
static member ConfigureProcessKitHostedProcess
104104
(services: IServiceCollection, name: string, configure: Action<HostedProcessOptions>)
105105
: IServiceCollection =
106-
ArgumentNullException.ThrowIfNull services
107-
ArgumentNullException.ThrowIfNull name
108-
ArgumentNullException.ThrowIfNull configure
106+
ArgumentNullException.ThrowIfNull(services, nameof services)
107+
ArgumentNullException.ThrowIfNull(name, nameof name)
108+
ArgumentNullException.ThrowIfNull(configure, nameof configure)
109109

110110
services.Configure(name, configure) |> ignore
111111
services
@@ -121,8 +121,8 @@ type HostedServiceCollectionExtensions =
121121
static member AddProcessKitHostedProcessHealthCheck
122122
(services: IServiceCollection, name: string)
123123
: IServiceCollection =
124-
ArgumentNullException.ThrowIfNull services
125-
ArgumentNullException.ThrowIfNull name
124+
ArgumentNullException.ThrowIfNull(services, nameof services)
125+
ArgumentNullException.ThrowIfNull(name, nameof name)
126126

127127
services.TryAddKeyedSingleton<HostedProcessHealthCheck>(
128128
name,

‎tests/ProcessKit.CSharp.Tests/DependencyInjectionTests.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ namespace ProcessKit.CSharp.Tests;
2323
[TestFixture]
2424
public class DependencyInjectionTests
2525
{
26+
private static void AssertNullParameter(string expected, Action call)
27+
{
28+
var exception = Assert.Throws<ArgumentNullException>(call);
29+
Assert.That(exception!.ParamName, Is.EqualTo(expected));
30+
}
31+
2632
private sealed class SingleValueConfiguration(string key, string? value) : IConfiguration
2733
{
2834
private readonly IConfigurationSection section = new SingleValueConfigurationSection(key, value);
@@ -157,6 +163,37 @@ public void AddProcessKitClient_rejects_a_null_configure_result_when_the_keyed_c
157163
Assert.That(exception!.ParamName, Is.EqualTo("configure"));
158164
}
159165

166+
[Test]
167+
public void DI_extension_null_guards_preserve_their_public_parameter_names()
168+
{
169+
var services = new ServiceCollection();
170+
171+
AssertNullParameter(
172+
"services",
173+
() => ServiceCollectionExtensions.AddProcessKit(null!));
174+
AssertNullParameter(
175+
"configure",
176+
() => services.AddProcessKit((Action<ProcessKitOptions>)null!));
177+
AssertNullParameter(
178+
"configuration",
179+
() => services.AddProcessKit((IConfiguration)null!));
180+
AssertNullParameter(
181+
"name",
182+
() => services.AddProcessKitClient(null!, "tool", client => client));
183+
AssertNullParameter(
184+
"program",
185+
() => services.AddProcessKitClient("tool", null!, client => client));
186+
AssertNullParameter(
187+
"configure",
188+
() => services.AddProcessKitClient("tool", "tool", (Func<CliClient, CliClient>)null!));
189+
AssertNullParameter(
190+
"configure",
191+
() => services.AddProcessKitGroup((Action<ProcessKitOptions>)null!));
192+
AssertNullParameter(
193+
"configuration",
194+
() => services.AddProcessKitGroup((IConfiguration)null!));
195+
}
196+
160197
[Test]
161198
public void AddProcessKit_with_a_configure_action_binds_ProcessKitOptions_defaults()
162199
{

‎tests/ProcessKit.Tests/DependencyInjectionTests.fs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@ type DependencyInjectionTests() =
9191

9292
Assert.That(thrown.ParamName, Is.EqualTo "command", verb)
9393

94+
let assertNullParameter (caseName: string) (expected: string) (call: unit -> unit) =
95+
let thrown =
96+
match Assert.Throws<ArgumentNullException>(Action call) with
97+
| null -> failwith $"Expected {caseName} to reject a null {expected}."
98+
| exceptionThrown -> exceptionThrown
99+
100+
Assert.That(thrown.ParamName, Is.EqualTo expected, caseName)
101+
94102
/// Poll until `pid` is no longer a live process (reaped/exited), or the deadline elapses.
95103
let waitProcessGone (pid: int) (deadlineMs: int) : Task<bool> =
96104
task {
@@ -240,6 +248,79 @@ type DependencyInjectionTests() =
240248

241249
Assert.That(thrown.ParamName, Is.EqualTo "services")
242250

251+
[<Test>]
252+
member _.``DI extension null guards preserve their public parameter names``() =
253+
let services = ServiceCollection() :> IServiceCollection
254+
let clientConfigure = Func<CliClient, CliClient>(id)
255+
256+
let calls: (string * string * (unit -> unit)) list =
257+
[ "AddProcessKit configure",
258+
"configure",
259+
(fun () ->
260+
ServiceCollectionExtensions.AddProcessKit(services, Unchecked.defaultof<Action<ProcessKitOptions>>)
261+
|> ignore)
262+
"AddProcessKit configuration",
263+
"configuration",
264+
(fun () ->
265+
ServiceCollectionExtensions.AddProcessKit(services, Unchecked.defaultof<IConfiguration>)
266+
|> ignore)
267+
"AddProcessKitClient services",
268+
"services",
269+
(fun () ->
270+
ServiceCollectionExtensions.AddProcessKitClient(
271+
Unchecked.defaultof<IServiceCollection>,
272+
"tool",
273+
"tool",
274+
clientConfigure
275+
)
276+
|> ignore)
277+
"AddProcessKitClient name",
278+
"name",
279+
(fun () ->
280+
ServiceCollectionExtensions.AddProcessKitClient(
281+
services,
282+
Unchecked.defaultof<string>,
283+
"tool",
284+
clientConfigure
285+
)
286+
|> ignore)
287+
"AddProcessKitClient program",
288+
"program",
289+
(fun () ->
290+
ServiceCollectionExtensions.AddProcessKitClient(
291+
services,
292+
"tool",
293+
Unchecked.defaultof<string>,
294+
clientConfigure
295+
)
296+
|> ignore)
297+
"AddProcessKitClient configure",
298+
"configure",
299+
(fun () ->
300+
ServiceCollectionExtensions.AddProcessKitClient(
301+
services,
302+
"tool",
303+
"tool",
304+
Unchecked.defaultof<Func<CliClient, CliClient>>
305+
)
306+
|> ignore)
307+
"AddProcessKitGroup configure",
308+
"configure",
309+
(fun () ->
310+
ServiceCollectionExtensions.AddProcessKitGroup(
311+
services,
312+
Unchecked.defaultof<Action<ProcessKitOptions>>
313+
)
314+
|> ignore)
315+
"AddProcessKitGroup configuration",
316+
"configuration",
317+
(fun () ->
318+
ServiceCollectionExtensions.AddProcessKitGroup(services, Unchecked.defaultof<IConfiguration>)
319+
|> ignore) ]
320+
321+
for caseName, expected, call in calls do
322+
assertNullParameter caseName expected call
323+
243324
[<Test>]
244325
member _.``the resolved runner is logger-aware when a logger factory is registered``() : Task =
245326
task {

‎tests/ProcessKit.Tests/HostedProcessHealthCheckTests.fs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,32 @@ type HostedProcessHealthCheckTests() =
6262
let healthCheck = provider.GetRequiredKeyedService<HostedProcessHealthCheck> name
6363
provider, hosted, service, healthCheck :> IHealthCheck
6464

65+
let assertNullParameter (caseName: string) (expected: string) (call: unit -> unit) =
66+
let thrown =
67+
match Assert.Throws<ArgumentNullException>(Action call) with
68+
| null -> failwith $"Expected {caseName} to reject a null {expected}."
69+
| exceptionThrown -> exceptionThrown
70+
71+
Assert.That(thrown.ParamName, Is.EqualTo expected, caseName)
72+
73+
[<Test>]
74+
member _.``Hosted-process health-check extension preserves its public null parameter names``() =
75+
let services = ServiceCollection() :> IServiceCollection
76+
77+
assertNullParameter "services" "services" (fun () ->
78+
HostedServiceCollectionExtensions.AddProcessKitHostedProcessHealthCheck(
79+
Unchecked.defaultof<IServiceCollection>,
80+
"worker"
81+
)
82+
|> ignore)
83+
84+
assertNullParameter "name" "name" (fun () ->
85+
HostedServiceCollectionExtensions.AddProcessKitHostedProcessHealthCheck(
86+
services,
87+
Unchecked.defaultof<string>
88+
)
89+
|> ignore)
90+
6591
[<Test>]
6692
member _.``AddProcessKitHostedProcessHealthCheck registers a keyed IHealthCheck resolvable by the same name``() =
6793
let services = ServiceCollection()

0 commit comments

Comments
 (0)