Allow logging of body without modifying the actual response #5628
Conversation
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpResponseBodyReader.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpResponseBodyReader.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpResponseBodyReader.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpResponseBodyReader.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpResponseBodyReader.cs
Show resolved
Hide resolved
| { | ||
| if (streamToReadFrom.CanSeek) | ||
| await WriteStreamToPipeAsync(streamToReadFrom, pipe.Writer, cancellationToken).ConfigureAwait(false); | ||
| }, CancellationToken.None); |
There was a problem hiding this comment.
Is CancellationToken.None preferred over using cancellationToken?
There was a problem hiding this comment.
I originally had it passed both to the inner task to write the stream to pipe, and the background task. My thoughts here is the inner task gets cancelled the thread will terminate anyway. I may be wrong about this. What do we prefer here?
There was a problem hiding this comment.
I'm not sure, we're using it in TcpEndpointProbesService like this, but not in FunctionInvokingChatClient .
After doing some reading, If believe it doesn't really matter in our scenario, given we are using it for the internal call and we don't have children tasks inside of this Task.Run(), and we're relying on the status of the cancelled task (cancelled vs faulted).
There seem to be opinions both for and against using it, so your call.
Closes #5511
Makes #4377 obsolete
Microsoft Reviewers: Open in CodeFlow