huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Address PR feedback

This commit is contained in:
Stephen Halter 2015-10-15 16:52:37 -07:00
parent 7b315d2470
commit 8e818e3549
2 changed files with 53 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs
View File

@ -42,7 +42,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
private bool _responseStarted; private bool _responseStarted;
private bool _keepAlive; private bool _keepAlive;
private bool _autoChunk; private bool _autoChunk;
private bool _applicationFailed; private Exception _applicationException;
public Frame(ConnectionContext context) : base(context) public Frame(ConnectionContext context) : base(context)
{ {
@ -79,6 +79,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
_responseStarted = false; _responseStarted = false;
_keepAlive = false; _keepAlive = false;
_autoChunk = false; _autoChunk = false;
_applicationException = null;
_requestHeaders.Reset(); _requestHeaders.Reset();
ResetResponseHeaders(); ResetResponseHeaders();
@ -183,10 +184,11 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
} }
finally finally
{ {
// Trigger FireOnStarting if ProduceStart hasn't been called yet. // Trigger OnStarting if it hasn't been called yet and the app hasn't
// We call it here, so it can go through our normal error handling // already failed. If an OnStarting callback throws we can go through
// and respond with a 500 if an OnStarting callback throws. // our normal error handling in ProduceEnd.
if (!_responseStarted) // https://github.com/aspnet/KestrelHttpServer/issues/43
if (!_responseStarted && _applicationException == null)
{ {
await FireOnStarting(); await FireOnStarting();
} }
@ -260,16 +262,16 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
} }
if (onStarting != null) if (onStarting != null)
{ {
foreach (var entry in onStarting) try
{ {
try foreach (var entry in onStarting)
{ {
await entry.Key.Invoke(entry.Value); await entry.Key.Invoke(entry.Value);
} }
catch (Exception ex) }
{ catch (Exception ex)
ReportApplicationError(ex); {
} ReportApplicationError(ex);
} }
} }
} }
@ -419,9 +421,11 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
await FireOnStarting(); await FireOnStarting();
if (_applicationFailed) if (_applicationException != null)
{ {
throw new ObjectDisposedException(typeof(Frame).FullName); throw new ObjectDisposedException(
"The response has been aborted due to an unhandled application exception.",
_applicationException);
} }
await ProduceStart(immediate, appCompleted: false); await ProduceStart(immediate, appCompleted: false);
@ -444,12 +448,12 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
private async Task ProduceEnd() private async Task ProduceEnd()
{ {
if (_applicationFailed) if (_applicationException != null)
{ {
if (_responseStarted) if (_responseStarted)
{ {
// We can no longer respond with a 500, so we simply close the connection. // We can no longer respond with a 500, so we simply close the connection.
_keepAlive = false; _requestProcessingStopping = true;
return; return;
} }
else else
@ -457,10 +461,6 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
StatusCode = 500; StatusCode = 500;
ReasonPhrase = null; ReasonPhrase = null;
// If OnStarting hasn't been triggered yet, we don't want to trigger it now that
// the app func has failed. https://github.com/aspnet/KestrelHttpServer/issues/43
_onStarting = null;
ResetResponseHeaders(); ResetResponseHeaders();
_responseHeaders.HeaderContentLength = "0"; _responseHeaders.HeaderContentLength = "0";
} }
@ -750,7 +750,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
private void ReportApplicationError(Exception ex) private void ReportApplicationError(Exception ex)
{ {
_applicationFailed = true; _applicationException = ex;
Log.ApplicationError(ex); Log.ApplicationError(ex);
} }
} }

42
test/Microsoft.AspNet.Server.KestrelTests/EngineTests.cs
View File

@ -550,7 +550,7 @@ namespace Microsoft.AspNet.Server.KestrelTests
[MemberData(nameof(ConnectionFilterData))] [MemberData(nameof(ConnectionFilterData))]
public async Task ThrowingResultsIn500Response(ServiceContext testContext) public async Task ThrowingResultsIn500Response(ServiceContext testContext)
{ {
var onStartingCallCount = 0; bool onStartingCalled = false;
var testLogger = new TestApplicationErrorLogger(); var testLogger = new TestApplicationErrorLogger();
testContext.Log = new KestrelTrace(testLogger); testContext.Log = new KestrelTrace(testLogger);
@ -560,7 +560,7 @@ namespace Microsoft.AspNet.Server.KestrelTests
var response = frame.Get<IHttpResponseFeature>(); var response = frame.Get<IHttpResponseFeature>();
response.OnStarting(_ => response.OnStarting(_ =>
{ {
onStartingCallCount++; onStartingCalled = true;
return Task.FromResult<object>(null); return Task.FromResult<object>(null);
}, null); }, null);
@ -597,7 +597,7 @@ namespace Microsoft.AspNet.Server.KestrelTests
"", "",
""); "");
Assert.Equal(2, onStartingCallCount); Assert.False(onStartingCalled);
Assert.Equal(2, testLogger.ApplicationErrorsLogged); Assert.Equal(2, testLogger.ApplicationErrorsLogged);
} }
} }
@ -760,7 +760,8 @@ namespace Microsoft.AspNet.Server.KestrelTests
[MemberData(nameof(ConnectionFilterData))] [MemberData(nameof(ConnectionFilterData))]
public async Task ThrowingInOnStartingResultsInFailedWritesAnd500Response(ServiceContext testContext) public async Task ThrowingInOnStartingResultsInFailedWritesAnd500Response(ServiceContext testContext)
{ {
var onStartingCallCount = 0; var onStartingCallCount1 = 0;
var onStartingCallCount2 = 0;
var failedWriteCount = 0; var failedWriteCount = 0;
var testLogger = new TestApplicationErrorLogger(); var testLogger = new TestApplicationErrorLogger();
@ -768,19 +769,28 @@ namespace Microsoft.AspNet.Server.KestrelTests
using (var server = new TestServer(async frame => using (var server = new TestServer(async frame =>
{ {
var onStartingException = new Exception();
var response = frame.Get<IHttpResponseFeature>(); var response = frame.Get<IHttpResponseFeature>();
response.OnStarting(_ => response.OnStarting(_ =>
{ {
onStartingCallCount++; onStartingCallCount1++;
throw new Exception(); throw onStartingException;
}, null);
response.OnStarting(_ =>
{
onStartingCallCount2++;
throw onStartingException;
}, null); }, null);
response.Headers.Clear(); response.Headers.Clear();
response.Headers["Content-Length"] = new[] { "11" }; response.Headers["Content-Length"] = new[] { "11" };
await Assert.ThrowsAsync<ObjectDisposedException>(async () => var writeException = await Assert.ThrowsAsync<ObjectDisposedException>(async () =>
await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello World"), 0, 11)); await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello World"), 0, 11));
Assert.Same(onStartingException, writeException.InnerException);
failedWriteCount++; failedWriteCount++;
}, testContext)) }, testContext))
{ {
@ -811,7 +821,9 @@ namespace Microsoft.AspNet.Server.KestrelTests
"", "",
""); "");
Assert.Equal(2, onStartingCallCount); Assert.Equal(2, onStartingCallCount1);
// The second OnStarting callback should not be called since the first failed.
Assert.Equal(0, onStartingCallCount2);
Assert.Equal(2, testLogger.ApplicationErrorsLogged); Assert.Equal(2, testLogger.ApplicationErrorsLogged);
} }
} }
@ -820,6 +832,9 @@ namespace Microsoft.AspNet.Server.KestrelTests
[MemberData(nameof(ConnectionFilterData))] [MemberData(nameof(ConnectionFilterData))]
public async Task ThrowingInOnCompletedIsLoggedAndClosesConnection(ServiceContext testContext) public async Task ThrowingInOnCompletedIsLoggedAndClosesConnection(ServiceContext testContext)
{ {
var onCompletedCalled1 = false;
var onCompletedCalled2 = false;
var testLogger = new TestApplicationErrorLogger(); var testLogger = new TestApplicationErrorLogger();
testContext.Log = new KestrelTrace(testLogger); testContext.Log = new KestrelTrace(testLogger);
@ -828,6 +843,12 @@ namespace Microsoft.AspNet.Server.KestrelTests
var response = frame.Get<IHttpResponseFeature>(); var response = frame.Get<IHttpResponseFeature>();
response.OnCompleted(_ => response.OnCompleted(_ =>
{ {
onCompletedCalled1 = true;
throw new Exception();
}, null);
response.OnCompleted(_ =>
{
onCompletedCalled2 = true;
throw new Exception(); throw new Exception();
}, null); }, null);
@ -850,7 +871,10 @@ namespace Microsoft.AspNet.Server.KestrelTests
"Hello World"); "Hello World");
} }
Assert.Equal(1, testLogger.ApplicationErrorsLogged); // All OnCompleted callbacks should be called even if they throw.
Assert.Equal(2, testLogger.ApplicationErrorsLogged);
Assert.True(onCompletedCalled1);
Assert.True(onCompletedCalled2);
} }
} }