Skip to content

Commit 2eb2eb4

Browse files
normjnormj
committed
Clean up
1 parent b7b51bd commit 2eb2eb4

File tree

5 files changed

+17
-56
lines changed

5 files changed

+17
-56
lines changed

‎Libraries/src/Amazon.Lambda.Core/ResponseStreaming/LambdaResponseStream.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,24 +82,24 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc
8282
/// <exception cref="NotSupportedException">Always thrown.</exception>
8383
public override long Position
8484
{
85-
get => throw new NotSupportedException("LambdaResponseStream does not support seeking.");
86-
set => throw new NotSupportedException("LambdaResponseStream does not support seeking.");
85+
get => throw new NotSupportedException($"{nameof(LambdaResponseStream)} does not support seeking.");
86+
set => throw new NotSupportedException($"{nameof(LambdaResponseStream)} does not support seeking.");
8787
}
8888

8989
/// <summary>Not supported.</summary>
9090
/// <exception cref="NotImplementedException">Always thrown.</exception>
9191
public override long Seek(long offset, SeekOrigin origin)
92-
=> throw new NotImplementedException("LambdaResponseStream does not support seeking.");
92+
=> throw new NotImplementedException($"{nameof(LambdaResponseStream)} does not support seeking.");
9393

9494
/// <summary>Not supported.</summary>
9595
/// <exception cref="NotImplementedException">Always thrown.</exception>
9696
public override int Read(byte[] buffer, int offset, int count)
97-
=> throw new NotImplementedException("LambdaResponseStream does not support reading.");
97+
=> throw new NotImplementedException($"{nameof(LambdaResponseStream)} does not support reading.");
9898

9999
/// <summary>Not supported.</summary>
100100
/// <exception cref="NotImplementedException">Always thrown.</exception>
101101
public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
102-
=> throw new NotImplementedException("LambdaResponseStream does not support reading.");
102+
=> throw new NotImplementedException($"{nameof(LambdaResponseStream)} does not support reading.");
103103

104104
/// <summary>
105105
/// Writes a sequence of bytes to the stream. Delegates to the async path synchronously.
@@ -116,7 +116,7 @@ public override void Flush() { }
116116
/// <summary>Not supported.</summary>
117117
/// <exception cref="NotSupportedException">Always thrown.</exception>
118118
public override void SetLength(long value)
119-
=> throw new NotSupportedException("LambdaResponseStream does not support SetLength.");
119+
=> throw new NotSupportedException($"{nameof(LambdaResponseStream)} does not support SetLength.");
120120
#endregion
121121
}
122122
}

‎Libraries/src/Amazon.Lambda.RuntimeSupport/Bootstrap/LambdaBootstrap.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -467,10 +467,7 @@ internal async Task InvokeOnceAsync(CancellationToken cancellationToken = defaul
467467
}
468468
finally
469469
{
470-
if (runtimeApiClient != null)
471-
{
472-
ResponseStreamFactory.CleanupInvocation(isMultiConcurrency);
473-
}
470+
ResponseStreamFactory.CleanupInvocation(isMultiConcurrency);
474471
invocation.Dispose();
475472
}
476473
};

‎Libraries/src/Amazon.Lambda.RuntimeSupport/Bootstrap/ResponseStreaming/ResponseStream.cs‎

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,14 @@ internal class ResponseStream
3636
// The live HTTP output stream, set by RawStreamingHttpClient when sending the streaming response.
3737
private Stream _httpOutputStream;
3838
private bool _disposedValue;
39-
private readonly SemaphoreSlim _httpStreamReady = new SemaphoreSlim(0, 1);
4039

41-
// The wait time is a sanity timeout to avoid waiting indefinitely if SerializeToStreamAsync is not called or takes too long to call.
42-
// Reality is that SerializeToStreamAsync should be called very quickly after CreateStream, so this timeout is generous to avoid false positives but still protects against hanging indefinitely.
40+
// The wait time is a sanity timeout to avoid waiting indefinitely if SetHttpOutputStreamAsync is not called or takes too long to call.
41+
// Reality is that SetHttpOutputStreamAsync should be called very quickly after CreateStream, so this timeout is generous to avoid false positives but still protects against hanging indefinitely.
4342
private readonly static TimeSpan _httpStreamWaitTimeout = TimeSpan.FromSeconds(30);
4443

44+
private readonly SemaphoreSlim _httpStreamReady = new SemaphoreSlim(0, 1);
4545
private readonly SemaphoreSlim _completionSignal = new SemaphoreSlim(0, 1);
4646

47-
4847
private static readonly byte[] PreludeDelimiter = new byte[8];
4948

5049
/// <summary>
@@ -145,7 +144,7 @@ public async Task WriteAsync(byte[] buffer, int offset, int count, CancellationT
145144
await _httpStreamReady.WaitAsync(_httpStreamWaitTimeout, cancellationToken);
146145
try
147146
{
148-
_logger.LogDebug($"Writing chunk of {count} bytes to HTTP stream.");
147+
_logger.LogDebug("Writing chunk to HTTP response stream.");
149148

150149
lock (_lock)
151150
{
@@ -183,8 +182,6 @@ internal void ReportError(Exception exception)
183182

184183
_hasError = true;
185184
_reportedError = exception;
186-
187-
188185
_isCompleted = true;
189186
}
190187
// Signal completion so RawStreamingHttpClient can write error trailers and finish
@@ -232,21 +229,8 @@ protected virtual void Dispose(bool disposing)
232229
try { _httpStreamReady.Release(); } catch (SemaphoreFullException) { /* Ignore if already released */ }
233230
_httpStreamReady.Dispose();
234231

235-
// Do NOT release or dispose _completionSignal here.
236-
//
237-
// When the handler uses "using var stream = ...", Dispose() runs during
238-
// stack unwinding BEFORE LambdaBootstrap's catch block can call ReportError().
239-
// If we release the signal here, RawStreamingHttpClient sees HasError=false
240-
// and writes the chunked terminator without error trailers, causing Lambda
241-
// to report Runtime.TruncatedResponse instead of the actual error.
242-
//
243-
// If we dispose the signal here, subsequent ReportError()/MarkCompleted()
244-
// calls and the WaitForCompletionAsync() in RawStreamingHttpClient will
245-
// throw ObjectDisposedException.
246-
//
247-
// The completion signal lifecycle is managed by MarkCompleted()/ReportError()
248-
// (which release it) and LambdaBootstrap (which awaits the send task after).
249-
// The SemaphoreSlim is a lightweight managed object that the GC will finalize.
232+
try { _completionSignal.Release(); } catch (SemaphoreFullException) { /* Ignore if already released */ }
233+
_completionSignal.Dispose();
250234
}
251235

252236
_disposedValue = true;

‎Libraries/test/Amazon.Lambda.RuntimeSupport.Tests/Amazon.Lambda.RuntimeSupport.IntegrationTests/ResponseStreamingTests.cs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,10 @@ public async Task UnhandledExceptionHandler()
6161
var evnts = await InvokeFunctionAsync(nameof(UnhandledExceptionHandler));
6262
Assert.True(evnts.Any());
6363

64-
var content = GetCombinedStreamContent(evnts);
65-
Assert.Contains("This method will fail", content);
66-
Assert.Contains("This is an unhandled exception", content);
67-
Assert.Contains("Lambda-Runtime-Function-Error-Type", content);
68-
Assert.Contains("InvalidOperationException", content);
69-
Assert.Contains("This is an unhandled exception", content);
70-
Assert.Contains("stackTrace", content);
64+
var completeEvent = evnts.Last() as InvokeWithResponseStreamCompleteEvent;
65+
Assert.Equal("InvalidOperationException", completeEvent.ErrorCode);
66+
Assert.Contains("This is an unhandled exception", completeEvent.ErrorDetails);
67+
Assert.Contains("stackTrace", completeEvent.ErrorDetails);
7168
}
7269

7370
private async Task<IEventStreamEvent[]> InvokeFunctionAsync(string handlerScenario)

‎Libraries/test/Amazon.Lambda.RuntimeSupport.Tests/Amazon.Lambda.RuntimeSupport.UnitTests/ResponseStreamTests.cs‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -217,23 +217,6 @@ public async Task ReportErrorAsync_NullException_ThrowsArgumentNull()
217217
Assert.Throws<ArgumentNullException>(() => stream.ReportError(null));
218218
}
219219

220-
[Fact]
221-
public async Task Dispose_DoesNotReleaseCompletionSignal()
222-
{
223-
var stream = new ResponseStream(Array.Empty<byte>());
224-
225-
var waitTask = stream.WaitForCompletionAsync();
226-
Assert.False(waitTask.IsCompleted);
227-
228-
stream.Dispose();
229-
230-
// Dispose should NOT release the completion signal — only MarkCompleted/ReportError should.
231-
// This prevents a race condition where the handler's "using" block releases the signal
232-
// before LambdaBootstrap can call ReportError, causing trailers to be omitted.
233-
var completed = await Task.WhenAny(waitTask, Task.Delay(TimeSpan.FromMilliseconds(200)));
234-
Assert.NotSame(waitTask, completed);
235-
}
236-
237220
[Fact]
238221
public async Task Dispose_CalledTwice_DoesNotThrow()
239222
{

0 commit comments

Comments
 (0)