Sanitize and centralize exception throws

[benaadams]

No description provided.

[benaadams]

@/Users/travis/build/aspnet/KestrelHttpServer/artifacts/logs/msbuild.logger.rsp
e (x64) 2.0.0
e (x64) 2.1.0-preview2-26130-04
ples/Http2SampleApp/Http2SampleApp.csproj...
  Restoring packages for /Users/travis/build/aspnet/KestrelHttpServer/benchmarks/Kestrel.Performance/Kestrel.Performance.csproj...
/Users/travis/.dotnet/sdk/2.2.0-preview1-008003/NuGet.targets(104,5): error : Unable to load the service index for source https://dotnet.myget.org/F/aspnetcore-dev/api/v3/index.json. [/Users/travis/build/aspnet/KestrelHttpServer/KestrelHttpServer.sln]
/Users/travis/.dotnet/sdk/2.2.0-preview1-008003/NuGet.targets(104,5): error :   An error occurred while sending the request. [/Users/travis/build/aspnet/KestrelHttpServer/KestrelHttpServer.sln]
/Users/travis/.dotnet/sdk/2.2.0-preview1-008003/NuGet.targets(104,5): error :   Couldn't resolve host name [/Users/travis/build/aspnet/KestrelHttpServer/KestrelHttpServer.sln]

[muratg]

@benaadams I'm assuming you're doing this for perf. Were you able to measure any perf gains?

[benaadams]

The throwing of exceptions is a bit haphazard in style; so it brings consistency.

Currently sometimes methods get an exception from BadHttpRequest then throw inline; other times BadHttpRequest throws for them; other times there's a second method in the class that gets the Exception and then throws out of line. There is even an non-static function that calls a static GetException meaning it has to go via the class _context.ThrowRequestRejected when its unnecessary because no class fields are used.

This moves everything to BadHttpRequest throwing; except the couple that need to check the local logging state.

There will likely be some mild pref improvements from a hotter instructions cache from not throwing inline; and sticking to the out-of-line throw generally used so the asm will be shorter per function; there won't be any big gains as most of the methods won't change to being inlinable as they are quite big.

So it more about being consistent in how exceptions are thrown. Some of the current methods are effected strongly by throwing out of line; which is why I was moving to that as the baseline.

[muratg]

@benaadams Could you rebase? @halter73 will look into taking this after.

[benaadams]

Rebased

[davidfowl]

@benaadams can you show what exceptions look like now?

[benaadams]

With line numbers

info: Microsoft.AspNetCore.Server.Kestrel[17]
      Connection id "0HLBQMKOSCLF1" bad request data: "Malformed request: invalid headers."
Microsoft.AspNetCore.Server.Kestrel.Core.BadHttpRequestException: Malformed request: invalid headers.
   at Microsoft.AspNetCore.Server.Kestrel.Core.BadHttpRequestException.Throw(RequestRejectionReason reason) in C:\GitHub\KestrelHttpServer\src\Kestrel.Core\BadHttpRequestException.cs:line 38
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.TryParseRequest(ReadResult result, Boolean& endConnection) in C:\GitHub\KestrelHttpServer\src\Kestrel.Core\Internal\Http\Http1Connection.cs:line 452
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) in C:\GitHub\KestrelHttpServer\src\Kestrel.Core\Internal\Http\HttpProtocol.cs:line 502
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequestsAsync[TContext](IHttpApplication`1 application) in C:\GitHub\KestrelHttpServer\src\Kestrel.Core\Internal\Http\HttpProtocol.cs:line 443

Or without line numbers

info: Microsoft.AspNetCore.Server.Kestrel[17]
      Connection id "0HLBQMKOSCLF1" bad request data: "Malformed request: invalid headers."
Microsoft.AspNetCore.Server.Kestrel.Core.BadHttpRequestException: Malformed request: invalid headers.
   at Microsoft.AspNetCore.Server.Kestrel.Core.BadHttpRequestException.Throw(RequestRejectionReason reason)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.TryParseRequest(ReadResult result, Boolean& endConnection)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) 
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequestsAsync[TContext](IHttpApplication`1 application) 

[benaadams]

Previous

info: Microsoft.AspNetCore.Server.Kestrel[17]
      Connection id "0HLBQMU06UGQF" bad request data: "Malformed request: invalid headers."
Microsoft.AspNetCore.Server.Kestrel.Core.BadHttpRequestException: Malformed request: invalid headers.
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.Http1Connection.TryParseRequest(ReadResult result, Boolean& endConnection) 
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) 
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequestsAsync[TContext](IHttpApplication`1 application)

G_M44841_IG22:
       B901000000           mov      ecx, 1
       E8D423E6FF           call     BadHttpRequestException:GetException(int):ref
       488BC8               mov      rcx, rax
       E8ECB6425F           call     CORINFO_HELP_THROW

G_M44841_IG23:
       B904000000           mov      ecx, 4
       E8C223E6FF           call     BadHttpRequestException:GetException(int):ref
       488BC8               mov      rcx, rax
       E8DAB6425F           call     CORINFO_HELP_THROW

G_M44841_IG24:
       B911000000           mov      ecx, 17
       E8B023E6FF           call     BadHttpRequestException:GetException(int):ref
       488BC8               mov      rcx, rax
       E8C8B6425F           call     CORINFO_HELP_THROW
       CC                   int3     

; Total bytes of code 754, prolog size 61 for method Http1Connection:TryParseRequest(struct,byref):bool:this

Post

G_M44844_IG22:
       B901000000           mov      ecx, 1
       E81420E6FF           call     BadHttpRequestException:Throw(int)

G_M44844_IG23:
       B904000000           mov      ecx, 4
       E80A20E6FF           call     BadHttpRequestException:Throw(int)

G_M44844_IG24:
       B911000000           mov      ecx, 17
       E80020E6FF           call     BadHttpRequestException:Throw(int)
       CC                   int3     

; Total bytes of code 722, prolog size 61 for method Http1Connection:TryParseRequest(struct,byref):bool:this

[davidfowl]

Fixed your spelling 😉