Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow developers to opt-out of sync XHR with feature policy #178

Closed
clelland opened this issue Dec 15, 2017 · 16 comments
Closed

Allow developers to opt-out of sync XHR with feature policy #178

clelland opened this issue Dec 15, 2017 · 16 comments

Comments

@clelland
Copy link
Contributor

Per #20, we may not be able to remove sync xhr completely from the platform, but maybe we can still do better than console warnings :)

I've written up a spec change (#177) to integrate XHR with feature policy, so that developers who have managed to avoid synchronous requests, and would like to ensure that their site stays sync-free (including all subresources and third-party frames), can opt in to a policy which disables it completely.

Under this policy, calling send() on a request with the synchronous flag set will appear to the script as a NetworkError.

@clelland
Copy link
Contributor Author

My understanding is that this idea had rough support from multiple vendors when it was brought up at TPAC last month (and specifically that throwing a NetworkError was preferred over, say an InvalidAccessError thrown on send()) I wasn't present for the discussion, though, so I don't have first-hand knowledge of it.

@RByers, @rniwa, @toddreifsteck -- Can anyone who was there in person confirm or deny my allegations here? :)

@annevk
Copy link
Member

annevk commented Dec 19, 2017

And maybe also come up with the supporting arguments? As past attempts to curtail synchronous XMLHttpRequest are all about throwing during open().

@clelland
Copy link
Contributor Author

Oops, yes, I meant that the original alternative proposal was InvalidAccessError thrown on open(), not send().

@RByers
Copy link

RByers commented Jan 10, 2018

From the discussion at TPAC (unfortunately not minuted due to WiFi being too flaky) there was talk of limiting the compat risk by choosing a failure mode that's similar to failures that most uses of XHR are likely to already handle (eg. network errors). We specifically want to reduce the severity of breakage in unprepared iframes (eg. AMP is prepared to enable this), and try to maximize the chance we could enable this mode by default at some point.

I don't have any hard data, but I suspect many uses of XHR are not prepared for open to throw an exception but are prepared for network errors. If that is true, then having open throw for this would greatly decrease the chance that embedders could apply this policy to arbitrary iframes, or that we could ever make this the default.

What about treating it similarly to an explicit abort? I see that developer errors like calling open on an already open XHR are treated like abort, so maybe it's reasonable for attempting a sync XHR when sync is disabled to be treated similarly?

I forget who it was that said they supported this argument at TPAC. /cc @dbaron @AngeloKai @travisleithead

@annevk
Copy link
Member

annevk commented Jan 10, 2018

Alright, I guess that's reasonable. I'm happy to throw on send() instead even if it's somewhat inconsistent with precedent. PR still needs some work on how all the various pieces fit together though.

@RuudZw
Copy link

RuudZw commented Jan 11, 2018

  1. The idea of removing an option is fundamentally flawed - nobody is forced to use an option. 2. Any coding in the web environment can lead to 'detrimental effects to the end user’s experience', including javascript itself. 3. I use synchronous XHR after the page has loaded, so then there is absolute no 'detrimental effects to the end user’s experience' - on the contrary. 4. I even use synchronous XHR during the loading of the pages, and I have never noticed any 'detrimental effects to the end user’s experience' - and that is with about ten years of experience. Conclusion: this idea has no ground at all.

@clelland
Copy link
Contributor Author

Hi @RuudZw -- thanks for the feedback. I can leave some of this to the spec authors (since https://xhr.spec.whatwg.org/#sync-warning has been in the spec for almost four years now -- see 575999a), but I would point out a couple of things about this particular proposal:

First, it's a developer opt-out, not a complete removal of synchronous XHR from the platform. If you need it for some reason, and you're happy with the performance, or mitigate any slowdowns through other means, then by all means continue to use it. You wouldn't have to make any changes to your site for this to continue to work.

Second, many people do find that synchronous actions slow their pages down, especially in third party content which they don't necessarily control, (and network speeds and conditions vary greatly in different parts of the world; what is fast for some users may be very slow in others, and javascript being single-threaded, a slow synchronous request necessarily pauses all scripts on that page until it completes or times out). This gives those site owners a way to ensure that their site can remain fast, without forcing that change onto the rest of the web.

If you still have concerns about the effects of removing synchronous XHR completely, #20 is a probably a good place to raise them.

@RuudZw
Copy link

RuudZw commented Jan 12, 2018

Hi clelland,
Thx for the speedy feedback. As you state: some people write crummy code. As I said: that can be done in javascript too (I even did it myself, sometimes) - say using alert(). No reason to remove alert() from javascript. I use syncXHR to get database data from the server and use this in the following code. This is highly simple and effective. I would be greatly helped with a suggestion for an alternative, since I'm no expert, just somebody who likes to use web applications to show other people things. BTW: I got here because FF said so - thx for the reroute.

@RuudZw
Copy link

RuudZw commented Jan 13, 2018

An addendum to my first post: 5. There is an alternative solution: add a time-out option to syncXHR, with a reasonable default value, of say 500 ms. That will force "careless" designers to make a conscious decision on it -and probable lead to the removal of most of the problematic use.

@domenic
Copy link
Member

domenic commented Jan 13, 2018

The problem is the users notice any lag longer than 16 ms, so that would be the appropriate timeout; 500 ms definitely hurts user experience. But very few servers, if any, are able to respond that fast.

@RByers
Copy link

RByers commented Jan 16, 2018

Other modern platforms like Android prohibit all synchronous network operations on the UI thread. Timeouts are hard to reason about (could mean your site works fine in testing, but not in deployment in countries with slower internet). The right solution is definitely to follow platforms like Android and just phase in a plan to require the use of either async APIs or workers for all network APIs. Recent improvements to JavaScript (like async/await) make asynchronous programming NEARLY as easy to do as synchronous programming, so IMHO the "synchronous is easier" really isn't a compelling argument.

BTW, you can better experience the potential user impact of relying on sync XHR by using DevTools network throttling with a very high latency - say 2000ms (not entirely unrealistic in many environments where Chrome is popular).

@eligrey
Copy link

eligrey commented Jan 16, 2018

Why specifically single-out sync XHR? There are many other UX responsiveness DoS mechanisms remaining.

I feel like if you want to use feature policy headers to control responsiveness, then we need a feature policy for blocking JavaScript execution entirely.

@AngeloKai
Copy link

Agreed with @RByers that sync-xhr typically causes hangs when the network is slow or the server is slow at responding. In both cases, developers coding in local environment or good office space don't notice those issues. I had a personal experience where developers working on a major top 100 sites don't notice the issue until I showed it with a network simulator.

Giving developer a way to opt-out of sync xhr is a great way to help developers avoid these performance pitfalls.

@RuudZw
Copy link

RuudZw commented Jan 19, 2018

RByers commented Jan 16, 2018
Other modern platforms like Android prohibit all synchronous network operations on the UI thread. Timeouts are hard to reason about (could mean your site works fine in testing, but not in deployment in countries with slower internet).
Reply:
May I suggest an alternative solution: do not use syncXHR if you don't know how to use it. Or any other technique that you're not familiar with. But do not mess with the tools of people who do. You want to deny me a tool because some people do not know how to use it. I do not appreciate that.

@RuudZw
Copy link

RuudZw commented Jan 19, 2018

@AngeloKai,
Or throw a time-out or an exception ... As I suggested.

@thw0rted
Copy link

thw0rted commented Feb 8, 2018

@eligrey genuinely curious, is there another (anti-)pattern that ties UI responsiveness to network conditions? The browser is inherently event-driven, and the most common events it deals with are user input and network call resolution. I'm not aware of any way to tell the UI thread to block completely until a user-input event occurs, and sync XHR is the only way I can think of to block until network resolution. I saw this argument made in the other issue I just replied to, but nobody seemed to have any examples.

@annevk annevk closed this as completed in 67a423f Mar 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants