Fix `NullReferenceException` in `HttpClientResponse.GetCharsetEncoding` #5881

andrewlock · 2024-08-12T13:54:31Z

Summary of changes

Fixes a NullReferenceException that occurred on some responses in .NET 6+

Reason for change

We were getting null reference exceptions in some cases

Implementation details

Fix the NRE, and add some more #nullable enable

Test coverage

Should probably have some :awkward-monkey:

andrewlock · 2024-08-12T14:02:26Z

tracer/src/Datadog.Trace/Agent/Transports/HttpClientResponse.cs

@@ -40,7 +42,7 @@ _response.Content.Headers.ContentEncoding.Count switch

        public Encoding GetCharsetEncoding()
        {
-            var charset = _response.Content.Headers.ContentType.CharSet;
+            var charset = _response.Content.Headers.ContentType?.CharSet;


This is the actual fix

datadog-ddstaging · 2024-08-12T14:52:21Z

Datadog Report

Branch report: andrew/fix-webrequest-bug
Commit report: 63feb56
Test service: dd-trace-dotnet

✅ 0 Failed, 346071 Passed, 1791 Skipped, 15h 14m 39.83s Total Time

andrewlock · 2024-08-12T14:54:10Z

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

Welch test with statistical test for significance of 5%
Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5881) - mean (71ms)  : 64, 78
     .   : milestone, 71,
    master - mean (75ms)  : 69, 81
     .   : milestone, 75,

    section CallTarget+Inlining+NGEN
    This PR (5881) - mean (1,073ms)  : 1048, 1098
     .   : milestone, 1073,
    master - mean (1,080ms)  : 1025, 1134
     .   : milestone, 1080,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5881) - mean (110ms)  : 106, 114
     .   : milestone, 110,
    master - mean (112ms)  : 105, 119
     .   : milestone, 112,

    section CallTarget+Inlining+NGEN
    This PR (5881) - mean (756ms)  : 733, 780
     .   : milestone, 756,
    master - mean (750ms)  : 730, 770
     .   : milestone, 750,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5881) - mean (93ms)  : 90, 97
     .   : milestone, 93,
    master - mean (93ms)  : 91, 95
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (5881) - mean (704ms)  : 687, 721
     .   : milestone, 704,
    master - mean (702ms)  : 686, 718
     .   : milestone, 702,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5881) - mean (192ms)  : 188, 196
     .   : milestone, 192,
    master - mean (192ms)  : 188, 195
     .   : milestone, 192,

    section CallTarget+Inlining+NGEN
    This PR (5881) - mean (1,171ms)  : 1144, 1198
     .   : milestone, 1171,
    master - mean (1,166ms)  : 1139, 1193
     .   : milestone, 1166,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5881) - mean (278ms)  : 272, 283
     .   : milestone, 278,
    master - mean (275ms)  : 272, 279
     .   : milestone, 275,

    section CallTarget+Inlining+NGEN
    This PR (5881) - mean (921ms)  : 900, 942
     .   : milestone, 921,
    master - mean (920ms)  : 894, 946
     .   : milestone, 920,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5881) - mean (265ms)  : 262, 269
     .   : milestone, 265,
    master - mean (264ms)  : 260, 268
     .   : milestone, 264,

    section CallTarget+Inlining+NGEN
    This PR (5881) - mean (906ms)  : 880, 933
     .   : milestone, 906,
    master - mean (902ms)  : 881, 922
     .   : milestone, 902,

andrewlock · 2024-08-13T01:09:16Z

Benchmarks Report for tracer 🐌

Benchmarks for #5881 compared to master:

2 benchmarks are faster, with geometric mean 1.150
All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`StartStopWithChild`	net6.0	7.73μs	43.2ns	270ns	0.0145	0.00724	0	5.43 KB
master	`StartStopWithChild`	netcoreapp3.1	10.2μs	56ns	322ns	0.0153	0.0102	0	5.62 KB
master	`StartStopWithChild`	net472	16.2μs	45.8ns	177ns	1.01	0.287	0.0878	6.07 KB
#5881	`StartStopWithChild`	net6.0	7.79μs	43.4ns	264ns	0.0155	0.00776	0	5.43 KB
#5881	`StartStopWithChild`	netcoreapp3.1	10.2μs	55.1ns	316ns	0.02	0.01	0	5.62 KB
#5881	`StartStopWithChild`	net472	16.4μs	79ns	410ns	1.03	0.315	0.105	6.06 KB

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	477μs	361ns	1.4μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	634μs	325ns	1.17μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	845μs	609ns	2.28μs	0.422	3.3 KB
#5881	`WriteAndFlushEnrichedTraces`	net6.0	467μs	252ns	943ns	0	2.7 KB
#5881	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	636μs	400ns	1.5μs	0	2.7 KB
#5881	`WriteAndFlushEnrichedTraces`	net472	850μs	663ns	2.57μs	0.422	3.3 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	210μs	1.21μs	10.2μs	0.209	18.45 KB
master	`SendRequest`	netcoreapp3.1	234μs	1.36μs	12.4μs	0.228	20.61 KB
master	`SendRequest`	net472	0.000288ns	0.000197ns	0.000764ns	0	0 b
#5881	`SendRequest`	net6.0	193μs	1.08μs	7.51μs	0.186	18.45 KB
#5881	`SendRequest`	netcoreapp3.1	222μs	1.2μs	9.05μs	0.221	20.61 KB
#5881	`SendRequest`	net472	0.00134ns	0.000485ns	0.00182ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	575μs	3.2μs	20.5μs	0.568	0	0	41.59 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	688μs	3.23μs	12.1μs	0.355	0	0	41.66 KB
master	`WriteAndFlushEnrichedTraces`	net472	870μs	4.53μs	21.7μs	8.13	2.57	0.428	53.31 KB
#5881	`WriteAndFlushEnrichedTraces`	net6.0	580μs	1.98μs	7.69μs	0.601	0	0	41.67 KB
#5881	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	681μs	3.59μs	24.4μs	0.353	0	0	41.84 KB
#5881	`WriteAndFlushEnrichedTraces`	net472	901μs	3.8μs	13.7μs	8.3	2.62	0.437	53.29 KB

Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	1.32μs	2.09ns	8.08ns	0.0145	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.67μs	1.66ns	6.42ns	0.0134	1.02 KB
master	`ExecuteNonQuery`	net472	1.97μs	1.29ns	5.01ns	0.157	987 B
#5881	`ExecuteNonQuery`	net6.0	1.38μs	2.26ns	8.75ns	0.0145	1.02 KB
#5881	`ExecuteNonQuery`	netcoreapp3.1	1.73μs	6.65ns	24ns	0.0132	1.02 KB
#5881	`ExecuteNonQuery`	net472	2.04μs	2.77ns	10.7ns	0.157	987 B

Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.25μs	1.28ns	4.95ns	0.0138	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.5μs	1.12ns	4.19ns	0.0135	976 B
master	`CallElasticsearch`	net472	2.5μs	2.33ns	9.03ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.34μs	0.777ns	3.01ns	0.0134	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.55μs	0.836ns	3.24ns	0.0138	1.02 KB
master	`CallElasticsearchAsync`	net472	2.68μs	3.42ns	13.2ns	0.166	1.05 KB
#5881	`CallElasticsearch`	net6.0	1.19μs	0.426ns	1.53ns	0.0137	976 B
#5881	`CallElasticsearch`	netcoreapp3.1	1.56μs	1.4ns	5.44ns	0.0132	976 B
#5881	`CallElasticsearch`	net472	2.56μs	8.81ns	34.1ns	0.158	995 B
#5881	`CallElasticsearchAsync`	net6.0	1.29μs	6.31ns	27.5ns	0.0129	952 B
#5881	`CallElasticsearchAsync`	netcoreapp3.1	1.58μs	3.11ns	12ns	0.0134	1.02 KB
#5881	`CallElasticsearchAsync`	net472	2.67μs	4.7ns	17.6ns	0.167	1.05 KB

Benchmarks.Trace.GraphQLBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5881

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net6.0	1.130	1,302.41	1,152.73

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.3μs	0.656ns	2.54ns	0.0137	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.58μs	0.975ns	3.65ns	0.0127	952 B
master	`ExecuteAsync`	net472	1.7μs	1.26ns	4.9ns	0.145	915 B
#5881	`ExecuteAsync`	net6.0	1.15μs	0.687ns	2.66ns	0.0129	952 B
#5881	`ExecuteAsync`	netcoreapp3.1	1.65μs	1.22ns	4.56ns	0.0124	952 B
#5881	`ExecuteAsync`	net472	1.81μs	1.47ns	5.48ns	0.145	915 B

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.14μs	1.65ns	6.4ns	0.0309	2.22 KB
master	`SendAsync`	netcoreapp3.1	5.09μs	1.48ns	5.55ns	0.038	2.76 KB
master	`SendAsync`	net472	7.69μs	1.42ns	5.13ns	0.499	3.15 KB
#5881	`SendAsync`	net6.0	4.15μs	3.1ns	12ns	0.0311	2.22 KB
#5881	`SendAsync`	netcoreapp3.1	5.21μs	2.16ns	8.37ns	0.0364	2.76 KB
#5881	`SendAsync`	net472	7.85μs	1.68ns	6.52ns	0.499	3.15 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.48μs	0.781ns	2.92ns	0.023	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.31μs	1.36ns	5.09ns	0.0219	1.64 KB
master	`EnrichedLog`	net472	2.63μs	1.66ns	6.2ns	0.249	1.57 KB
#5881	`EnrichedLog`	net6.0	1.54μs	3.24ns	12.5ns	0.0228	1.64 KB
#5881	`EnrichedLog`	netcoreapp3.1	2.14μs	2.32ns	8.35ns	0.0225	1.64 KB
#5881	`EnrichedLog`	net472	2.55μs	1.2ns	4.64ns	0.249	1.57 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	117μs	110ns	425ns	0.0589	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	121μs	347ns	1.35μs	0	0	4.28 KB
master	`EnrichedLog`	net472	150μs	227ns	878ns	0.679	0.226	4.46 KB
#5881	`EnrichedLog`	net6.0	117μs	241ns	932ns	0.0583	0	4.28 KB
#5881	`EnrichedLog`	netcoreapp3.1	122μs	238ns	923ns	0	0	4.28 KB
#5881	`EnrichedLog`	net472	151μs	238ns	890ns	0.673	0.224	4.46 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	3.11μs	2.68ns	10.4ns	0.0313	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.17μs	1.21ns	4.53ns	0.0292	2.2 KB
master	`EnrichedLog`	net472	4.8μs	0.656ns	2.27ns	0.32	2.02 KB
#5881	`EnrichedLog`	net6.0	2.95μs	1.49ns	5.76ns	0.0311	2.2 KB
#5881	`EnrichedLog`	netcoreapp3.1	4.08μs	1.3ns	4.86ns	0.0286	2.2 KB
#5881	`EnrichedLog`	net472	4.75μs	1.25ns	4.85ns	0.319	2.02 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`SendReceive`	net6.0	1.36μs	1.35ns	5.24ns	0.0162	0	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.85μs	1.16ns	4.5ns	0.0156	0	1.14 KB
master	`SendReceive`	net472	2.18μs	1.34ns	5.03ns	0.183	0.00109	1.16 KB
#5881	`SendReceive`	net6.0	1.32μs	0.985ns	3.82ns	0.0158	0	1.14 KB
#5881	`SendReceive`	netcoreapp3.1	1.82μs	0.867ns	3.13ns	0.0154	0	1.14 KB
#5881	`SendReceive`	net472	2.21μs	10.9ns	46.1ns	0.183	0.00109	1.16 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.76μs	0.797ns	2.98ns	0.0221	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	4.05μs	1.11ns	4.01ns	0.0204	1.65 KB
master	`EnrichedLog`	net472	4.49μs	3.1ns	12ns	0.323	2.04 KB
#5881	`EnrichedLog`	net6.0	2.77μs	1.19ns	4.62ns	0.0229	1.6 KB
#5881	`EnrichedLog`	netcoreapp3.1	3.89μs	1.22ns	4.73ns	0.0214	1.65 KB
#5881	`EnrichedLog`	net472	4.42μs	4.01ns	15.5ns	0.323	2.04 KB

Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	393ns	0.149ns	0.576ns	0.00814	576 B
master	`StartFinishSpan`	netcoreapp3.1	584ns	0.261ns	1.01ns	0.0078	576 B
master	`StartFinishSpan`	net472	615ns	0.441ns	1.71ns	0.0917	578 B
master	`StartFinishScope`	net6.0	487ns	0.562ns	2.18ns	0.00972	696 B
master	`StartFinishScope`	netcoreapp3.1	723ns	0.375ns	1.45ns	0.00908	696 B
master	`StartFinishScope`	net472	835ns	0.456ns	1.77ns	0.105	658 B
#5881	`StartFinishSpan`	net6.0	396ns	0.558ns	2.16ns	0.00815	576 B
#5881	`StartFinishSpan`	netcoreapp3.1	568ns	0.771ns	2.88ns	0.00763	576 B
#5881	`StartFinishSpan`	net472	579ns	0.141ns	0.487ns	0.0916	578 B
#5881	`StartFinishScope`	net6.0	502ns	0.886ns	3.43ns	0.00985	696 B
#5881	`StartFinishScope`	netcoreapp3.1	686ns	1.27ns	4.93ns	0.00932	696 B
#5881	`StartFinishScope`	net472	916ns	0.861ns	3.11ns	0.104	658 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5881

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0	1.171	704.96	601.77

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	705ns	0.647ns	2.51ns	0.00985	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	973ns	1.35ns	5.22ns	0.00939	696 B
master	`RunOnMethodBegin`	net472	1.03μs	0.535ns	2.07ns	0.104	658 B
#5881	`RunOnMethodBegin`	net6.0	601ns	0.319ns	1.24ns	0.00982	696 B
#5881	`RunOnMethodBegin`	netcoreapp3.1	901ns	0.612ns	2.37ns	0.00948	696 B
#5881	`RunOnMethodBegin`	net472	1.14μs	0.251ns	0.972ns	0.104	658 B

…g` (#5881) ## Summary of changes Fixes a `NullReferenceException` that occurred on some responses in .NET 6+ ## Reason for change We were getting null reference exceptions in some cases ## Implementation details Fix the NRE, and add some more `#nullable enable` ## Test coverage Should probably have some :awkward-monkey:

…g` (#5881 -> v2) (#5887) ## Summary of changes Fixes a `NullReferenceException` that occurred on some responses in .NET 6+ ## Reason for change We were getting null reference exceptions in some cases ## Implementation details Fix the NRE, and add some more `#nullable enable` ## Test coverage Should probably have some :awkward-monkey:

andrewlock added type:bug area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) identified-by:telemetry labels Aug 12, 2024

andrewlock requested a review from a team as a code owner August 12, 2024 13:54

andrewlock commented Aug 12, 2024

View reviewed changes

andrewlock force-pushed the andrew/fix-webrequest-bug branch from e055b6a to 7d68083 Compare August 12, 2024 14:06

Fix null reference exception in HttpClientResponse.GetCharsetEncoding

63feb56

andrewlock force-pushed the andrew/fix-webrequest-bug branch from 7d68083 to 63feb56 Compare August 12, 2024 14:26

andrewlock requested a review from a team as a code owner August 12, 2024 14:26

kevingosse approved these changes Aug 13, 2024

View reviewed changes

pierotibou approved these changes Aug 13, 2024

View reviewed changes

andrewlock merged commit fde6a98 into master Aug 13, 2024
62 of 66 checks passed

andrewlock deleted the andrew/fix-webrequest-bug branch August 13, 2024 11:10

github-actions bot added this to the vNext-v3 milestone Aug 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `NullReferenceException` in `HttpClientResponse.GetCharsetEncoding` #5881

Fix `NullReferenceException` in `HttpClientResponse.GetCharsetEncoding` #5881

andrewlock commented Aug 12, 2024

andrewlock Aug 12, 2024

datadog-ddstaging bot commented Aug 12, 2024 •

edited

Loading

andrewlock commented Aug 12, 2024

andrewlock commented Aug 13, 2024

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #5881

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #5881

Raw results

Fix NullReferenceException in HttpClientResponse.GetCharsetEncoding #5881

Fix NullReferenceException in HttpClientResponse.GetCharsetEncoding #5881

Conversation

andrewlock commented Aug 12, 2024

Summary of changes

Reason for change

Implementation details

Test coverage

andrewlock Aug 12, 2024

Choose a reason for hiding this comment

datadog-ddstaging bot commented Aug 12, 2024 • edited Loading

Datadog Report

andrewlock commented Aug 12, 2024

Execution-Time Benchmarks Report ⏱️

andrewlock commented Aug 13, 2024

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #5881

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #5881

Raw results

Fix `NullReferenceException` in `HttpClientResponse.GetCharsetEncoding` #5881

Fix `NullReferenceException` in `HttpClientResponse.GetCharsetEncoding` #5881

datadog-ddstaging bot commented Aug 12, 2024 •

edited

Loading