Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Null HttpContext with long polling #1644

Closed
JamesNK opened this issue Mar 19, 2018 · 6 comments
Closed

Null HttpContext with long polling #1644

JamesNK opened this issue Mar 19, 2018 · 6 comments
Assignees
Labels
3 - Done cost: S Will take up to 2 days to complete

Comments

@JamesNK
Copy link
Member

JamesNK commented Mar 19, 2018

A hub method that accesses the HttpContext can get null instead if the method was invoked while the poll request ended.

This has shown up with HubConnectionTests.ClientCanSendHeaders(transportType: LongPolling) on OSX build.

https://travis-ci.org/aspnet/SignalR/jobs/355210173#L1309

[xUnit.net 00:00:06.1150930]     Microsoft.AspNetCore.SignalR.Client.FunctionalTests.HubConnectionTests.ClientCanSendHeaders(transportType: LongPolling) [FAIL]
Failed   Microsoft.AspNetCore.SignalR.Client.FunctionalTests.HubConnectionTests.ClientCanSendHeaders(transportType: LongPolling)
Error Message:
 Microsoft.AspNetCore.SignalR.Client.HubException : An unexpected error occurred invoking 'GetHeaderValues' on the server. InvalidOperationException: Unable to get HttpContext from request.
Stack Trace:
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
@analogrelay
Copy link
Contributor

Yeah, this is a tricky one because the connection is long-living but the HttpContext is only associated with a single Send, Poll, EventSource stream or WebSocket request. Perhaps we need to just remove the HttpContext entirely and copy any relevant data into the ConnectionContext.

@analogrelay analogrelay added this to the 2.1.0-preview2 milestone Mar 19, 2018
@analogrelay analogrelay added cost: M Will take 3 - 5 days to complete status: Needs Design cost: L Will take 6 - 10 days to complete and removed cost: M Will take 3 - 5 days to complete labels Mar 19, 2018
@davidfowl
Copy link
Member

davidfowl commented Mar 20, 2018

Since we are decoupled from the actual hub invocation process we're going to remove the HttpContext from the connection features. Instead, we're going to need to copy data from the HttpContext (Headers and QueryString).

@analogrelay
Copy link
Contributor

Also Connection info (IP Addresses, ports)

@davidfowl
Copy link
Member

@anurse I had an evil idea. Still set an HttpContext but with the copied data. The benefit is that it just works with all of the APIs that take an http context (like the auth methods) but it's harmless since it isn't valid as write to or read from (the bodies), and it has the properties that people want.

@analogrelay
Copy link
Contributor

analogrelay commented Mar 20, 2018

It's not that crazy. We'd need to poison, or null-out things like the Body stream. It would take a lot of effort to get it right though, possibly more than just copying the things we know we want.

@davidfowl
Copy link
Member

Yep, I'm going to do this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 - Done cost: S Will take up to 2 days to complete
Projects
None yet
Development

No branches or pull requests

3 participants