whatwg / xhr

XMLHttpRequest Standard
https://xhr.spec.whatwg.org/
Other
311 stars 128 forks source link

Abandon hope of removing sync XHR from the web platform? #20

Open foolip opened 9 years ago

foolip commented 9 years ago

https://xhr.spec.whatwg.org/#sync-warning

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's experience. (This is a long process that takes many years.) Developers must not pass false for the async argument when the JavaScript global environment is a document environment. User agents are strongly encouraged to warn about such usage in developer tools and may experiment with throwing an InvalidAccessError exception when it occurs.

There are some Blink use counters for sync XHR: https://www.chromestatus.com/metrics/feature/timeline/popularity/465 (measure only) https://www.chromestatus.com/metrics/feature/timeline/popularity/472 (deprecated) https://www.chromestatus.com/metrics/feature/timeline/popularity/581 (deprecated) https://www.chromestatus.com/metrics/feature/timeline/popularity/677 (async for comparison)

This does not look at all promising. There's a slight downwards trend, but starting from >2% it's hard to see it dropping to the point where removal is possible.

foolip commented 9 years ago

Some discussion (by me) at https://code.google.com/p/chromium/issues/detail?id=392311#c17

caitp commented 9 years ago

Software developers generally don't like warnings in their terminals (or dev consoles). If they know why they shouldn't use it, they're less likely to ignore it. If warnings show up, their users are slightly more Likely to report issues with their product.

There are good reasons to remove this use pattern from the platform, so folks should not give up on the idea.

foolip commented 9 years ago

At this level of usage, it doesn't seem likely that console warning will be able to drive down usage enough so that sync XHR can be removed. If console warnings is the only tool in our tool box, I think giving up hope is appropriate.

hallvors commented 9 years ago

Probably discussed to death, but if the browser can just keep its UI responsive does it really matter if a script author sometimes makes his/her page freeze by a sync call to an unreliable or slow server?

glittle commented 9 years ago

To me, this is very strange. When I click on a link to go to another page, the browser UI become unresponsive as it goes to get that page.

I have a few cases in my code that I need the browser to wait while it talks to the server using XHR. The user has just clicked a button, and is expecting a short wait.

If I can't do that call synchronously, I'll have to build an artificial approach to freeze the UI until the response is received!

I can live with a "warning" in the developer terminal, but to remove this feature from the spec is going to hurt a lot!

Timeouts can be used to ensure the UI isn't locked for too long!

marcoscaceres commented 9 years ago

@glittle, can you explain how you "need" to pause the browser? Maybe show us some code? It can't be that 98% of other developers don't have this issue, but you do. Can you explain what is so critical in the interaction with the server that the user is forced to not use the browser or the page?

glittle commented 9 years ago

In this case, the call for data is sent down a number of levels, and an answer is expected. If the server call is async, there won't be an answer returned. I can see that that sounds like a bad approach in an async world. I guess I'll have to bite the bullet and redesign each of the levels of code to use async calls, and keep the user entertained while they wait!

marcoscaceres commented 9 years ago

@glittle thanks for the additional details. Best of luck with the refactor! Your users, and local Browser Vendor, will thank you for it :)

In the slim chance you are using ES6, there are ways of keeping your code looking like it's sync, but actually running async (with generators and promises).

annevk commented 9 years ago

@hallvors yes, because the user experience of such an application sucks. Sites need to be motivated to use good practices so they don't become abandonware.

@foolip apart from the desire to remove, anyone using this also should really reconsider. Is the warning really that harmful?

foolip commented 9 years ago

@annevk this is the console warning in Blink with by far the highest usage, around 2%: https://www.chromestatus.com/metrics/feature/timeline/popularity/581

There's trade-off here, between training web developers to ignore console warnings because they happen too often and for things that aren't actually going to break, and nudging people away from sync XHR even if it will never be removed. 1% would be better than 2%, after all.

What the spec says on the topic (quoted above) goes too far, I think. I don't think sync XHR is "in the process of being removed from the web platform" and no browser will be able to throw InvalidAccessError any time soon. If it were made a bit more tentative like "sync XHR on the main thread is bad for the user experience, browsers please show console warnings and developers please stop using it, if usage is eventually low enough maybe we'll remove it" that would be better.

annevk commented 9 years ago

Given the discussion in https://lists.w3.org/Archives/Public/public-webapps/2014JanMar/thread.html#msg232 I guess we can consider alternative wording. I think the wording was based on showModelDialog()...

shakyjake commented 9 years ago

I don't think we'll see a drop off until support for FormData in Web Workers improves (currently only Chrome and Opera support it). Until then there's no real alternative for developers.

foolip commented 9 years ago

That's a connection I was not aware of, which uses of sync XHR can be replaced by FormData in Web Workers?

shakyjake commented 9 years ago

Well I can only speak from personal experience - I'm currently using sync XHR with FormData for a drag & drop file upload system. Async causes issues with script timeouts on the server when uploading multiple large files so I need to keep it synchronous. Web Workers seemed like the ideal solution for this but the poor support for FormData means I can't use it yet.

foolip commented 9 years ago

I'm not sure I understand, does the server timeout not result in an error event in the async case like it should? Is this to work around an implementation bug, or is there something actually broken in the specs here?

shakyjake commented 9 years ago

No, it's just that using async in this situation causes multiple uploads to start at (roughly) the same time, slowing the time required for each file to finish. Error events all happen (and ar received) as expected.

The spec is fine:)

foolip commented 9 years ago

Oh, so I suppose it's a matter of bookkeeping, using sync XHR saves you the trouble of keeping track of pending uploads and starting them when one is finished?

mreshadi commented 9 years ago

If sync XHR is taken away, what alternative mechanism are you proposing to achieve same effect. We have a javascript-streaming optimization which breaks the js into smaller pieces and loads the missing ones on demand. It has been very effective in reducing the size of js and removing unused parts (http://www.instartlogic.com/products/performance/streaming/javascript-streaming). This optimizations needs to pause the js execution until network response is received and as of now, only sync XHR can achieve this.

Browser is constantly touted as a platform that can replace even OS. In a platform scenario, there will all kinds of people offering different tools / value. Removal of sync XHR because it slows the page, assumes that the web community only consists of browser gurus and naive web developers who can shoot themselves in the foot!!

If the page is slow, google search ranking will anyways punish the page, why do you want to dumb down the browser for the rest of us?! How does this helps browser to become a true platform, and not a nanny framework for less capable programmers?!

foolip commented 9 years ago

@reshadi, this is a bug about accepting that we cannot remove sync XHR, and having the spec reflect that.

mreshadi commented 9 years ago

@foolip thanks for clarification. I think I miss read it. I'm glad this decision is made! :)

JamesDunne commented 8 years ago

There are several valid and useful patterns that require the use of synchronous XHR for which there is no alternative available. This proposal of deprecating synchronous XHR and the inclusion of a warning in developer console is an unnecessary scare tactic. Yes, I understand the warning is aimed at developers who don't know better with the presumably good intention to incite action aimed at improving the "general" user experience once a page is loaded, but to smack the hands of the capable and take away a powerful tool for which there is no alternative is not an acceptable solution.

foolip commented 8 years ago

@JamesDunne, this is a bug about accepting that we cannot remove sync XHR, and having the spec reflect that.

TheJaredWilcurt commented 8 years ago

Can we get some examples put in place for better practices designed to be as easy and simple to understand as possible, so those baby programmers this warning is meant for can actually update their code accordingly. Meanwhile put a harder to understand message for senior developers to understand that this is a false warning and XHR Sync, like Disco, will never die.

hallvors commented 8 years ago

Isn't this issue handled by promises and other work on making JS itself more "asyncish"? In other words, I would consider an implementation somewhat obnoxious if it warned me against doing something like

new Promise(function(resolve, reject){
    var foo = new XMLHttpRequest();
    foo.open('GET', 'bar.txt', false);
    foo.onload = function(){resolve(foo.responseText)}
    foo.onerror = function(){reject(foo}
    foo.send();
}).then( /* do the work that required waiting for the response */ );

FWIW, I think I'd vote for allowing synchronous APIs in the web platform and solving the problem with browsers being so single-threaded by focusing on that problem.

hallvors commented 8 years ago

(I admit that I have not tested that code to see if browsers implement sync-xhr-in-promise the way I'd expect them to do it ;). Perhaps we should add a test or five in due course?)

JamesDunne commented 8 years ago

@hallvors Promises are orthogonal to the issue here. The foo.send() line in your code above produces blocking behavior, i.e. JavaScript execution will pause at that line until the request is completed, making the rest of the Promise wrapper rather pointless in terms of asynchrony.

hallvors commented 8 years ago

Bugger. Well, thanks for clearing that up. Too bad.

tyoshino commented 8 years ago

Updates about Chromium metrics (with metric names as the box sometimes does not get set):

agilehands commented 8 years ago

Feeling relieved to know that sync XHR is going to stay!

@marcoscaceres Sync XHR is important in advertising platform, perhaps it falls in <2% cases. Here goes a practical example. Please note, I tried to make the example "readable" by changing the actual codes as it might be too specific to the domain knowledge.

--- begin example -- Current problem: Supporting 3rd party tag based demand in MoPub SDK w/o Sync XHR.

Here is how mopub javascripts/html page looks like if XYZ ad network wants to add their tag:

<script>
// XYZ script here. It adds a script tag dynamically
// in the document with the ad request url expecting a JSOONP response 
// OR makes an Async XHR request.
XYZ.loadAd();

// mopub assumes, that if loadAd fails to deliver an ad,
// it should call MoPub.fail() here. so the next script won't be loaded
// or will have no effect.

// BUT, the script tag, even w/o async mode, 
// will be executed after the next script tag is evaluated. 
// I.e. following script tag will be executed either before or in parallel. 
// there is NO way to stop the JS engine to render the next script tag 
// w/o sync XHR, as far as I know.
</script>

// The following script tag is added by MoPub SDK before loading the full html page
// into the browser.
// So we don't know what is in there beforehand. We can only inspected what they added.
<script>
// It is last block, so MoPub assumes that if reached here then
// an ad is delivered.
MoPub.AdReceivedSuccess().

// if  XYZ.loadAd() fails to deliver an ad and but if
// the above call is made before that, then MoPub 
// will count a false impression. That is really bad for business.
</script>

--- end example ---

Hope that gives you at least one use case of Sync XHR. However, If MoPub had a better/another way e.g callback/delegate etc, Sync XHR wouldn't be needed.

But till then, in such many cases where we cannot change a big player - such 3rd party integration needs Sync XHR.

foolip commented 8 years ago

It sounds like the problem is that if MoPub.AdReceivedSuccess() is reached it's simply assumed that the previous ad was loaded successfully. Even with all the sync XHR in the world, why would that necessarily be true?

The example is a bit too abstract to give advise about, but in principle it seems straightforward to load ads without sync XHR and only report them as successfully downloaded and rendered when they really are. Can you fill in the blanks for me?

agilehands commented 8 years ago

There is no blank, you got it right. As you said: "only report them as successfully downloaded and rendered when they really are", this should be the ideal solution. But as long as MoPub keeps adding the last script segment "automatically" and doesn't provide a mechanism to callback on success, Sync XHR is the only way.

Few notes:

  1. There is no such thing like MoPub.AdReceivedSuccess() or MoPub object. I used these for clarity instead of their javascript codes and url redirections.
  2. At present I can "swizzle" some mopub methods found by inspecting and make it work w/o Sync XHR. But its not reliable as mopub can change their implementation anytime.
foolip commented 8 years ago

OK, sounds like the hypothetical MoPub needs to make some changes and that sync XHR is a workaround. Maybe you can tell the real MoPub that you're getting console warnings because of them and ask them pretty please to change their model :)

agilehands commented 8 years ago

exactly, thats a very good suggestion (y)!

BourgeoisBear commented 8 years ago

The spec should definitely remove this bogus deprecation message. I have wasted so much time reading through all of these issues for something which is probably never going away.

Worse still, when anyone brings up the fact that sometimes (albeit rarely) sync is appropriate, @annevk gives a curt knee-jerk dismissal instead of offering solid proof of why sync is never neccessary, and an explanation of why it is so dangerous that it is worth breaking working code over.

foolip commented 8 years ago

@BourgeoisBear, was it the deprecation message in Blink (Chrome) that led you here? Not that it matters much, but the one in Blink is what prompted me to file this issue to begin with, and I'm wondering if it would be best to remove or leave it, all things considered.

domenic commented 8 years ago

I think it would be good to leave it. For every 1 developer who gets outraged and believes that their use case is truly unsolvable without janking the user experience, and comes to complan here, there may be 10 or 100 more who see the message and improve their user's experience, and alongside it the health of the web ecosystem.

BourgeoisBear commented 8 years ago

Chrome debug window it is!

I strongly lean towards not having a message (unless sync actually is being slated for removal). If sync is staying, the message should probably go. There are so many ways an unskilled dev can shoot himself in the foot. If the plan is to make a message for each one of those ways, the debug console is eventually going to be so choked up with noise that people will just disregard it.

If the message has to stay, changing the verbiage from "deprecation" to "try to use async" would be a great improvement.

On Thu, Mar 17, 2016 at 3:17 PM, Philip Jägenstedt <notifications@github.com

wrote:

@BourgeoisBear https://github.com/BourgeoisBear, was it the deprecation message in Blink (Chrome) that led you here? Not that it matters much, but the one in Blink is what prompted me to file this issue to begin with, and I'm wondering if it would be best to remove or leave it, all things considered.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/whatwg/xhr/issues/20#issuecomment-198065167

domenic commented 8 years ago

I think @foolip is more pessimistic than most, but there are still many who would like to remove it from the platform---like we did with showModalDialog, despite similar people claiming that it was crucial to their site. Especially given Chrome's recent focus on interventions, I can imagine such a push to take place for sync XHR in the near-ish future.

BourgeoisBear commented 8 years ago

People use javascript for things other than public-facing websites. Breaking "user space" (so to speak) is a big deal. If there is some security issue at stake here, I would understand, but no one has raised one in this discussion. If the reason for removing it is "I think you should have used async there buddy!", that is a crap reason to break user space.

On Thu, Mar 17, 2016 at 3:36 PM, Domenic Denicola notifications@github.com wrote:

I think @foolip https://github.com/foolip is more pessimistic than most, but there are still many who would like to remove it from the platform---like we did with showModalDialog, despite similar people claiming that it was crucial to their site. Especially given Chrome's recent focus on interventions, I can imagine such a push to take place for sync XHR in the near-ish future.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/whatwg/xhr/issues/20#issuecomment-198071244

foolip commented 8 years ago

It is true that I'm pessimistic about any "wait and see" approach to this, but together with a more active plan to get rid of this, the deprecation message could still make sense. What's still missing is someone to formulate and execute such a plan.

sideshowbarker commented 8 years ago

I think it would be good to leave it.

I do also.

For every 1 developer who gets outraged and believes that their use case is truly unsolvable without janking the user experience, and comes to complain here, there may be 10 or 100 more who see the message and improve their user's experience, and alongside it the health of the web ecosystem.

Yes. Speaking from my perspective in development of messages for the HTML checker, and seeing the results, I know messages about deprecated-but-still-supported misfeatures can make a difference during the time while browsers have to continue to support them and we continue to be stuck with them until more developers learn how bad they are for users and quit using them.

So in the interim, such messages have the potential to eventually reduce the usage to the point where we can actually remove the misfeature.

In this case I realize the odds might seem long right now, and the usage may end up remaining high enough forever that we never actually get there—but if we just give up on doing things like continuing to raise developer awareness of the problem by emitting console messages, we’ll never know if we might have been able to reduce usage below the threshold.

BourgeoisBear commented 8 years ago

The opinion voiced by the WHATWG members here seems to be: "If only we can educate these fools who are still using async: false, then usage will drop low enough to where we can remove it!"

No one uses async: false by accident. It is not the default. No one stumbles into it. No one wants to use async: false. Developers use async: false because they have been painted into a corner where it becomes the lesser of two evils. My particular corner involved a 3rd party library that I would rather not fork. Others' corners seem to involve 3rd party libraries that they cannot fork.

The usage is not low because you are spreading the gospel. The usage is low because, like a lifeboat or a spare tire, you only need async: false on a rare occasion.

There would be far less griping over this if one of the proponents took the time to write-up why async: false is so evil. Not just slow! unresponsive! [Duh], but an actual, technical, non-insulting article on why this feature is better dead than alive.

foolip commented 8 years ago

@BourgeoisBear, there isn't a shared WHATWG stance on this, I'm involved in the WHATWG and filed this issue precisely because I doubt that usage can drop by enough, at least unless there are additional efforts to understand and drive down usage.

It's interesting that you mention 3rd party libraries, which seems to have been the problem in https://github.com/whatwg/xhr/issues/20#issuecomment-187188271 as well. What do the details here look like? Are those 3rd party libraries using document.write, so that some init data for them is fetched using sync XHR?

BourgeoisBear commented 8 years ago

In my particular case, I’m using a 3rd-party date-time-timezone widget. On init, I can supply it with a “get current time” callback that is supposed to return the current date/time/timezone (for when the “Now” button inside the widget is pressed). The widget takes care of updating the DOM to reflect the new date/time (calendar, sliders, dropdowns, etc).

Here’s what pushed me towards async: false:

From a little web research, it seems as though async: false is used a lot for file-upload scenarios and browser-based desktop-style applications.

https://bugs.jquery.com/ticket/11013 https://core.trac.wordpress.org/ticket/33349

I’m a little fuzzy on the rationale for getting rid of this feature (aside from it blocking the main thread and not being the most desirable practice). Can anyone point me to a deeper discussion on why this has to go?

foolip commented 8 years ago

OK, so there's a "get current time" callback that the widget calls every time the user clicks a "Now" button, and inside that callback you do sync XHR? Or do you do sync XHR before the widget becomes visible and use a cached value + interpolation?

Have you looked into delivering the server's time in <html data-server-time="2016-03-21T10:25:25Z"> or similar? And if it's a long-lived page and time drift is a problem, maybe sync with async XHR once in a while?

Whatever is the case, while the sync XHR request is in flight, it won't be possible to interact with anything else on the page, right? That's the problem, it doesn't go any deeper AFAIK.

BourgeoisBear commented 8 years ago

@foolip:

The sync XHR call is inside the "get current time" callback that I provide to the widget.

The page is long-lived. The time polling idea is close. Minor drawback is that the server will have to constantly source multiple polls--instead of sourcing singles on an as-needed basis. Major drawback is that the "now" call takes the user-selected time zone as an input (like America/Pacific), and the server returns "now" in that time zone. If I polled in UTC and cached it, I would have to do time zone conversion client-side. JS native date/time facilities are anemic, so I'd have to pull in another dependency (moment.js or the like). Or, I could pull "now" in all time zones and cache them all. Still far less elegant than setting async to false.

If the sole problem we're trying to solve here is excessive blocking in the rendering thread, why does it have to be something as drastic as full removal? Putting a tighter ceiling on the timeout would have the same effect, wouldn't break as much code, and would give the developer an opportunity handle the timeout gracefully. It couldn't possibly be as bad as just silently ignoring the async flag so that things which were originally developed to run synchronously mysteriously stop doing so.

foolip commented 8 years ago

What kind of date handling is done of the server that there are no good APIs for in the browser? I guess it's not simply adding/subtracting a timezone offset, so something to do with human readable strings like "America/Pacific", maybe?

If you know up front the offset between performance.now() and the server-side time in UTC, then I doubt you'd need to sync it more than say once a week/month or so. How much does performance.now() drift, in practice?

As for just lowering the timeout, in order to ensure that scripted animations are not janky, the timeout would have to be before the next animation frame callback, so never more than 16ms. Even on the fastest of networks, that's not going to be reliable.

BourgeoisBear commented 8 years ago

It uses human readable time zones since there is more to time zones than offsets (different time zones can temporarily share offsets, like for daylight savings time, then the offsets will diverge). Moment.js does these, but pulling in a new dependency (the quality of which I cannot vouch for) is not as nice as changing a flag.

As for the performance.now() thing, it is certainly doable, but then we’re adding state to a process which was previously stateless.

foolip commented 8 years ago

The kind of jank I'm talking about is any script that's using requestAnimationFrame to drive an animation, as that animation would stop while the sync XHR is in progress. Maybe your page doesn't have any animation like that, but the browser can't assume that in the general case. And even if there are no animations, you won't be able to select text, follow links or otherwise interact with the page.

Demo: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4020

If it's too fast to see the problem, just emulate a slower network or XHR html.spec.whatwg.org instead.

It sounds like you know how to avoid using sync XHR here, but that it's more work. It's not a case where you're truly stuck with sync XHR because of a 3rd party library, as is undoubtedly the case sometimes.

BourgeoisBear commented 8 years ago

With enough time and effort, yeah, I can program around this, but I have many customers with many different systems. If one day they all upgrade their browsers, and sync XHR stops working, that's not a good day! Lots of billable hours? Yes. Billable hours that I want? No.

I'm having a hard time understanding the value-system that gives priority to a cosmetic concern (which can be self-remedied) over not-breaking existing code.

Look at how many terminal-based applications that we use as developers. As a platform, the terminal lacks a great many things, yet it is stable, which lets the developer focus on the problem instead of the platform. When you have a platform that makes changes lightly (windows, gnome, android, iOS, etc.), instead of being able to double-down on the quality and functionality of your program, you end up in a perpetual struggle to keep it functioning correctly across all of the different versions of the platform. It's stagnation. It's IE all over again.