The issue is handled for Firefox 11+ but not for chrome.
Here is the code (line 589 of 2.2.0.js):
// Firefox 11+ doesn't allow sync XHR withCredentials: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#withCredentials
var isFirefox11OrGreater = signalR._.firefoxMajorVersion(window.navigator.userAgent) >= 11,
asyncAbort = !!connection.withCredentials && isFirefox11OrGreater;
Since I'm not using Firefox the var gets set to false, which then does a synchronous request causing a warning to be displayed in the console everytime I leave a webpage.
suggested fix:
use the signalR async default here or create a new default for this exact situation
Can you please elaborate on what you mean by "use the signlR async default..."
Not sure what version of chrome this started in, but here's a "fix" to at least get the warning to go away.
signalR._ = {
...
chromeMajorVersion: function (userAgent) {
// Chrome user agents: http://useragentstring.com
var matches = userAgent.match(/Chrome\/(\d+)/);
if (!matches || !matches.length || matches.length < 2) {
return 0;
}
return parseInt(matches[1], 10 /* radix */);
},
...
var isFirefox11OrGreater = signalR._.firefoxMajorVersion(window.navigator.userAgent) >= 11,
isChrome58OrGreater = signalR._.chromeMajorVersion(window.navigator.userAgent) >= 58,
asyncAbort = (!!connection.withCredentials && isFirefox11OrGreater) || isChrome58OrGreater;
...
if (isFirefox11OrGreater || isChrome58OrGreater) {
...
connection.stop(asyncAbort);
Is there any particular reason for making abort
explictly synchronous?
If there are any signalR service issues and if the abort call takes terribly long time because of that, user experience is really detrimental - we faced this issue, whenever user navigates to other pages, page is blocked waiting for abort
to finish.
Could we do async by default? (make asyncAbort
true
always?)
@yaananth My guess is that there isn't any real reason, since it has worked this way for Firefox for such a long time. I agree that it should always be true.
Looks like we need to check for firefox version to handle a different issue. Not sure that we need to check for chrome version.
It's possible that it's there to make sure we disconnect the connect before leaving the page, so that connection doesn't remain open.
So, the client makes stop call to ensure disconnection, if the client doesn't do it (it's possible it won't if we change this to async, which I think we should), the server would timeout in 30 seconds (default) and disconnects, what are we saving here? Extra 30 seconds or so of keeping orphan connection open vs leading it to detrimental user experience if there are any issues?
I vote for asynchronous always true.
Anyone have any concerns?
@yaananth I also don't see anything detrimental either. There really isn't much of a difference and should just be async.
Put up a PR to serve as a starting point for the fix. This issue has been outstanding for some time, so thought it would help grease the wheels.
@halter73 @NTaylorMullen - do you remember why the abort request was send synchronously from the JavaScript SignalR client? Was it to make sure that the server actually received the abort
request and invoked OnDisconnected
or there was a different reason?
@moozzyk you're correct. When a browser tab is closing we wanted to hang the tab to ensure the request went through. A lot of the time the tab would close, request would get cancelled, then the server would be relying on its own heartbeat/keepalive to kill the connection. Not sure why we felt that letting the server kill off connections on its own wasn't sufficient 馃槃; thinking back hanging the browser tab seems like a much worse experience
It does look like we all agree to make it async by default, awesome! thanks @KyleScharnhorst for the PR!
We may need to make this backwards compatible though.
@moozzyk
Is it then better to just do setTimeout
(keeping it still sync) and push it to the event loop?
What are were we trying to solve here? @NTaylorMullen
If it's 1, we could do setTimeout
I believe, still doing sync (please feel free to correct me)
If it's 2, I don't think we can avoid it, and quite frankly I don't think we care much about this, right?
@yaananth - I don't want to redesign the client in a servicing release. If we decide to take this we will likely make abort async and do fire an forget (it's 1 from your list) and hope for the best with a flag somewhere the user can pass to get the old behavior (or vice versa - it is to be decided which behavior is going to be default).
@moozzyk Thanks! I would certainly push for making it async
by default and having an option to fallback if needed (since sync can cause bad experience and using sync on UI thread is anti-pattern). @KyleScharnhorst would you take care of this as well or should I may be take this up sometime later?
@yaananth I'd be happy to do this.
Is there already a place where user flags are handled?
May be add a param for signalR = function (url, qs, logging)
in this file?
May be let the new parameter be an object of different flags, instead of a just a boolean.
Typically the sync XHR didn't block closing the browser tab for very long. @NTaylorMullen is right that we originally did this because some browsers would not even send an async XHR in window.unload/beforeunload. We went through a lot of effort to ensure that OnDisconnected got called quickly after either a browser tab or window was closed.
IIRC, most browsers (including firefox >= 11) would send the async XHR during unload even if the entire app was exiting. I wouldn't be surprised if by now all modern browsers will send the async XHR. If so, it might be worth changing the default with a quirks flag to get back the old behavior.
https://xhr.spec.whatwg.org/#synchronous-flag
Synchronous XMLHttpRequest outside of workers is in the process of being removed from the web platform as it has detrimental effects to the end user鈥檚 experience. (This is a long process that takes many years.) Developers must not pass false for the async argument when current global object is a Window object. User agents are strongly encouraged to warn about such usage in developer tools and may experiment with throwing an InvalidAccessError exception when it occurs
I don't think anyone can guarantee that abort gets called with async, same thing when my computer shutdowns suddenly. There is server timeout which is 30 seconds by default, so the connection will eventually get cleaned up. I really think it's worth making it default and yes support old behavior through a flag. I have seen pretty bad experience on chrome due to this, I would certainly push for making it default.
@KyleScharnhorst just checking, would you be working on this?
Maybe options on start https://github.com/SignalR/SignalR/blob/master/src/Microsoft.AspNet.SignalR.Client.JS/jquery.signalR.core.js#L423?
Sorry guys, something came up. I won't be able to work on this.
Hi all,
Unsure if this is still in the pipeline anywhere, but this bit us somewhat today. We have a server that connects to a signalr hub at a separate server. This, for whatever reason, was failing to acknowledge the 'abort' request from signalr, meaning that trying to leave the page on Chrome or Edge completely froze the site, pending that response. Firefox correctly handled this asynchronously and continued to leave the page, but users on Chrome or Edge were completely unable to use the site. Overriding the 'async' parameter in jquery.signalr.js allowed Chrome and Edge to function.
Whilst the root of the problem is trying to establish why our server wouldn't correctly send an abort response, I'm still not entirely sure what synchronously waiting for the abort response is achieving as I can't tell that it's actually using the response for anything, and appears to be able to block the window unload.
Any further thoughts on this point?
The reason for the synchronous abort ajax request was to ensure browsers actually sent the abort reqeust triggered via the window (before)unload event when users closed the browser window or tab.
If the abort request wasn't sent, Hub.OnDisconnected and similar would not be fired on the server until a rather lengthy timeout completed well after users left the webapp.
The reason for the synchronous abort ajax request was to ensure browsers actually sent the abort reqeust triggered via the window (before)unload event when users closed the browser window or tab.
If the abort request wasn't sent, Hub.OnDisconnected and similar would not be fired on the server until a rather lengthy timeout completed well after users left the webapp.
Is this still a concern? Even if we don't want to assume that async is safe to call on unload from all browsers, could we perhaps have an override on $.connection.hub to allow forcing async abort calls? That way, I could handle this how I'd prefer, as in my use case the signalr feature is only a nicety, certainly not worth the risk of completely freezing up the UI on a window unload.
This issue has been closed as part of issue clean-up as described in https://blogs.msdn.microsoft.com/webdev/2018/09/17/the-future-of-asp-net-signalr/. If you're still encountering this problem, please feel free to re-open and comment to let us know! We're still interested in hearing from you, the backlog just got a little big and we had to do a bulk clean up to get back on top of things. Thanks for your continued feedback!
Most helpful comment
Not sure what version of chrome this started in, but here's a "fix" to at least get the warning to go away.