Skip to content

Commit 1e7ba25

Browse files
stevejgordonstevejgordon
committed
Improve response handling and add LeaveOpen property
Add `LeaveOpen` property to `StreamResponse` and `TransportResponse` classes for flexible stream management.
1 parent c030d63 commit 1e7ba25

File tree

5 files changed

+83
-77
lines changed

5 files changed

+83
-77
lines changed

‎src/Elastic.Transport/Components/Pipeline/DefaultResponseBuilder.cs‎

Lines changed: 47 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -224,70 +224,69 @@ private async ValueTask<TResponse> SetBodyCoreAsync<TResponse>(bool isAsync,
224224
details.ResponseBodyInBytes = bytes;
225225
}
226226

227-
var isStreamResponse = typeof(TResponse) == typeof(StreamResponse);
227+
if (SetSpecialTypes<TResponse>(mimeType, bytes, responseStream, requestData.MemoryStreamFactory, out var r)) return r;
228228

229-
using (isStreamResponse ? Stream.Null : responseStream ??= Stream.Null)
230-
{
231-
if (SetSpecialTypes<TResponse>(mimeType, bytes, responseStream, requestData.MemoryStreamFactory, out var r)) return r;
229+
if (details.HttpStatusCode.HasValue &&
230+
requestData.SkipDeserializationForStatusCodes.Contains(details.HttpStatusCode.Value))
231+
return null;
232232

233-
if (details.HttpStatusCode.HasValue &&
234-
requestData.SkipDeserializationForStatusCodes.Contains(details.HttpStatusCode.Value))
235-
return null;
233+
var serializer = requestData.ConnectionSettings.RequestResponseSerializer;
236234

237-
var serializer = requestData.ConnectionSettings.RequestResponseSerializer;
235+
TResponse response;
236+
if (requestData.CustomResponseBuilder != null)
237+
{
238+
var beforeTicks = Stopwatch.GetTimestamp();
238239

239-
TResponse response;
240-
if (requestData.CustomResponseBuilder != null)
241-
{
242-
var beforeTicks = Stopwatch.GetTimestamp();
240+
if (isAsync)
241+
response = await requestData.CustomResponseBuilder
242+
.DeserializeResponseAsync(serializer, details, responseStream, cancellationToken)
243+
.ConfigureAwait(false) as TResponse;
244+
else
245+
response = requestData.CustomResponseBuilder
246+
.DeserializeResponse(serializer, details, responseStream) as TResponse;
243247

244-
if (isAsync)
245-
response = await requestData.CustomResponseBuilder
246-
.DeserializeResponseAsync(serializer, details, responseStream, cancellationToken)
247-
.ConfigureAwait(false) as TResponse;
248-
else
249-
response = requestData.CustomResponseBuilder
250-
.DeserializeResponse(serializer, details, responseStream) as TResponse;
248+
var deserializeResponseMs = (Stopwatch.GetTimestamp() - beforeTicks) / (Stopwatch.Frequency / 1000);
249+
if (deserializeResponseMs > OpenTelemetry.MinimumMillisecondsToEmitTimingSpanAttribute && OpenTelemetry.CurrentSpanIsElasticTransportOwnedHasListenersAndAllDataRequested)
250+
Activity.Current?.SetTag(OpenTelemetryAttributes.ElasticTransportDeserializeResponseMs, deserializeResponseMs);
251251

252-
var deserializeResponseMs = (Stopwatch.GetTimestamp() - beforeTicks) / (Stopwatch.Frequency / 1000);
253-
if (deserializeResponseMs > OpenTelemetry.MinimumMillisecondsToEmitTimingSpanAttribute && OpenTelemetry.CurrentSpanIsElasticTransportOwnedHasListenersAndAllDataRequested)
254-
Activity.Current?.SetTag(OpenTelemetryAttributes.ElasticTransportDeserializeResponseMs, deserializeResponseMs);
252+
return response;
253+
}
255254

255+
// TODO: Handle empty data in a nicer way as throwing exceptions has a cost we'd like to avoid!
256+
// ie. check content-length (add to ApiCallDetails)? Content-length cannot be retrieved from a GZip content stream which is annoying.
257+
try
258+
{
259+
if (requiresErrorDeserialization && TryGetError(details, requestData, responseStream, out var error) && error.HasError())
260+
{
261+
response = new TResponse();
262+
SetErrorOnResponse(response, error);
256263
return response;
257264
}
258265

259-
// TODO: Handle empty data in a nicer way as throwing exceptions has a cost we'd like to avoid!
260-
// ie. check content-length (add to ApiCallDetails)? Content-length cannot be retrieved from a GZip content stream which is annoying.
261-
try
262-
{
263-
if (requiresErrorDeserialization && TryGetError(details, requestData, responseStream, out var error) && error.HasError())
264-
{
265-
response = new TResponse();
266-
SetErrorOnResponse(response, error);
267-
return response;
268-
}
266+
if (!requestData.ValidateResponseContentType(mimeType))
267+
return default;
269268

270-
if (!requestData.ValidateResponseContentType(mimeType))
271-
return default;
269+
var beforeTicks = Stopwatch.GetTimestamp();
272270

273-
var beforeTicks = Stopwatch.GetTimestamp();
271+
if (isAsync)
272+
response = await serializer.DeserializeAsync<TResponse>(responseStream, cancellationToken).ConfigureAwait(false);
273+
else
274+
response = serializer.Deserialize<TResponse>(responseStream);
274275

275-
if (isAsync)
276-
response = await serializer.DeserializeAsync<TResponse>(responseStream, cancellationToken).ConfigureAwait(false);
277-
else
278-
response = serializer.Deserialize<TResponse>(responseStream);
276+
var deserializeResponseMs = (Stopwatch.GetTimestamp() - beforeTicks) / (Stopwatch.Frequency / 1000);
279277

280-
var deserializeResponseMs = (Stopwatch.GetTimestamp() - beforeTicks) / (Stopwatch.Frequency / 1000);
278+
if (deserializeResponseMs > OpenTelemetry.MinimumMillisecondsToEmitTimingSpanAttribute && OpenTelemetry.CurrentSpanIsElasticTransportOwnedHasListenersAndAllDataRequested)
279+
Activity.Current?.SetTag(OpenTelemetryAttributes.ElasticTransportDeserializeResponseMs, deserializeResponseMs);
281280

282-
if (deserializeResponseMs > OpenTelemetry.MinimumMillisecondsToEmitTimingSpanAttribute && OpenTelemetry.CurrentSpanIsElasticTransportOwnedHasListenersAndAllDataRequested)
283-
Activity.Current?.SetTag(OpenTelemetryAttributes.ElasticTransportDeserializeResponseMs, deserializeResponseMs);
281+
if (!response.LeaveOpen)
282+
responseStream.Dispose();
284283

285-
return response;
286-
}
287-
catch (JsonException ex) when (ex.Message.Contains("The input does not contain any JSON tokens"))
288-
{
289-
return default;
290-
}
284+
return response;
285+
}
286+
catch (JsonException ex) when (ex.Message.Contains("The input does not contain any JSON tokens"))
287+
{
288+
responseStream.Dispose();
289+
return default;
291290
}
292291
}
293292

‎src/Elastic.Transport/Components/TransportClient/HttpRequestInvoker.cs‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -154,43 +154,43 @@ private async ValueTask<TResponse> RequestCoreAsync<TResponse>(bool isAsync, Req
154154
ex = e;
155155
}
156156

157-
var isStreamResponse = typeof(TResponse) == typeof(StreamResponse);
157+
TResponse response;
158158

159-
using (isStreamResponse ? DiagnosticSources.SingletonDisposable : receivedResponse)
159+
try
160160
{
161-
TResponse response;
161+
if (isAsync)
162+
response = await requestData.ConnectionSettings.ProductRegistration.ResponseBuilder.ToResponseAsync<TResponse>
163+
(requestData, ex, statusCode, responseHeaders, responseStream, mimeType, contentLength, threadPoolStats, tcpStats, cancellationToken)
164+
.ConfigureAwait(false);
165+
else
166+
response = requestData.ConnectionSettings.ProductRegistration.ResponseBuilder.ToResponse<TResponse>
167+
(requestData, ex, statusCode, responseHeaders, responseStream, mimeType, contentLength, threadPoolStats, tcpStats);
162168

163-
try
164-
{
165-
if (isAsync)
166-
response = await requestData.ConnectionSettings.ProductRegistration.ResponseBuilder.ToResponseAsync<TResponse>
167-
(requestData, ex, statusCode, responseHeaders, responseStream, mimeType, contentLength, threadPoolStats, tcpStats, cancellationToken)
168-
.ConfigureAwait(false);
169-
else
170-
response = requestData.ConnectionSettings.ProductRegistration.ResponseBuilder.ToResponse<TResponse>
171-
(requestData, ex, statusCode, responseHeaders, responseStream, mimeType, contentLength, threadPoolStats, tcpStats);
169+
// Defer disposal of the response message
170+
if (response is StreamResponse sr)
171+
sr.Finalizer = () => receivedResponse.Dispose();
172172

173-
// Defer disposal of the response message
174-
if (response is StreamResponse sr)
175-
sr.Finalizer = () => receivedResponse.Dispose();
173+
if (!OpenTelemetry.CurrentSpanIsElasticTransportOwnedAndHasListeners || (!(Activity.Current?.IsAllDataRequested ?? false)))
174+
return response;
176175

177-
if (!OpenTelemetry.CurrentSpanIsElasticTransportOwnedAndHasListeners || (!(Activity.Current?.IsAllDataRequested ?? false)))
178-
return response;
176+
var attributes = requestData.ConnectionSettings.ProductRegistration.ParseOpenTelemetryAttributesFromApiCallDetails(response.ApiCallDetails);
179177

180-
var attributes = requestData.ConnectionSettings.ProductRegistration.ParseOpenTelemetryAttributesFromApiCallDetails(response.ApiCallDetails);
178+
if (attributes is null) return response;
181179

182-
if (attributes is null) return response;
180+
foreach (var attribute in attributes)
181+
Activity.Current?.SetTag(attribute.Key, attribute.Value);
183182

184-
foreach (var attribute in attributes)
185-
Activity.Current?.SetTag(attribute.Key, attribute.Value);
183+
// Unless indicated otherwise by the TransportResponse, we've now handled the response stream, so we can dispose of the HttpResponseMessage
184+
// to release the connection.
185+
if (!response.LeaveOpen)
186+
receivedResponse.Dispose();
186187

187-
return response;
188-
}
189-
catch
190-
{
191-
receivedResponse.Dispose(); // if there's an exception, ensure we always release the response so that the connection is freed.
192-
throw;
193-
}
188+
return response;
189+
}
190+
catch
191+
{
192+
receivedResponse.Dispose(); // if there's an exception, ensure we always release the response so that the connection is freed.
193+
throw;
194194
}
195195
}
196196

‎src/Elastic.Transport/Components/TransportClient/HttpWebRequestInvoker.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,9 @@ private async ValueTask<TResponse> RequestCoreAsync<TResponse>(bool isAsync, Req
190190
}
191191
}
192192

193+
if (!response.LeaveOpen)
194+
receivedResponse.Dispose();
195+
193196
return response;
194197
}
195198
catch

‎src/Elastic.Transport/Responses/Special/StreamResponse.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ public StreamResponse(Stream body, string? mimeType)
4040
MimeType = mimeType ?? string.Empty;
4141
}
4242

43+
internal override bool LeaveOpen => true;
44+
4345
/// <summary>
4446
/// Disposes the underlying stream.
4547
/// </summary>

‎src/Elastic.Transport/Responses/TransportResponse.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace Elastic.Transport;
88

99
/// <summary>
1010
/// A response from an Elastic product including details about the request/response life cycle. Base class for the built in low level response
11-
/// types, <see cref="StringResponse"/>, <see cref="BytesResponse"/>, <see cref="DynamicResponse"/> and <see cref="VoidResponse"/>
11+
/// types, <see cref="StringResponse"/>, <see cref="BytesResponse"/>, <see cref="DynamicResponse"/>, <see cref="StreamResponse"/> and <see cref="VoidResponse"/>
1212
/// </summary>
1313
public abstract class TransportResponse<T> : TransportResponse
1414
{
@@ -34,5 +34,7 @@ public abstract class TransportResponse
3434
public override string ToString() => ApiCallDetails?.DebugInformation
3535
// ReSharper disable once ConstantNullCoalescingCondition
3636
?? $"{nameof(ApiCallDetails)} not set, likely a bug, reverting to default ToString(): {base.ToString()}";
37+
38+
internal virtual bool LeaveOpen { get; } = false;
3739
}
3840

0 commit comments

Comments
 (0)