search

linkedin / dustjs

Asynchronous Javascript templating for the browser and server
http://dustjs.com
MIT License
2.91k stars 479 forks source link

Incomplete loading with iOS mobile safari #509

Closed calvintwr closed 10 years ago

calvintwr commented 10 years ago

Please refer to this stackoverflow issue for full details: http://stackoverflow.com/questions/26352839/linkedin-dustjs-on-node-express-ios-safari

This may not be purely a dust issue, but I am unable to pinpoint where the fault it. Effectively, I have removed all modules and revert the app back to a standard Express setup. The problem persists with dust, but is gone when i replaced the view engine with EJS.

In short, on the first clean GET request, page loading could not be completed when using iOS Safari. Inspector is showing that latter resources are not getting a response. They, will receive the response after 2 minutes.

There seem to be a limit as to the page size before this happens, meaning to say, as I include more js or images after the threshold, they will run into "not getting a response".

sethkinast commented 10 years ago

What does an example view you are trying to render look like?

aredridel commented 10 years ago

Yeah, examples would be super useful in diagnosing this; how it's integrated matters, since everything wants to monkey patch.

calvintwr commented 10 years ago

Sorry for the lack of an example. Here is it.

I have removed all partials and some dust helpers. Here's the file:

https://gist.github.com/calvintwr/179453c85d5b1f0c9029

All of the javascript are standard libraries except signUpCode.js. When i take out the js from signUpCode and put it inside the dust file, it seem to work fine. So does leaving it and removing some other javascript libraries. It seems that with more external files, the threshold is more likely to be reached.

Here's the code for that:

https://gist.github.com/calvintwr/a3a433b410dbcdec7530

calvintwr commented 10 years ago

I have reduced my app to a smaller error reproducible form:

https://github.com/calvintwr/hashiontag_dev/tree/ioserror

aredridel commented 10 years ago

Okay then!

This only happens in in Mobile Safari? Other browsers are unaffected?

calvintwr commented 10 years ago

Yup. So far its been tested on all major browsers.

The cases I have tested to fail are iOS6 through to iOS8.

aredridel commented 10 years ago

Wow, since this is going through HTTP, there's not much that can be specific to dust here. I'd be looking for low-level problems -- looking at the packets of HTTP requests and seeing what's different in those cases.

calvintwr commented 10 years ago

I do have the headers comparing between the ok case, versus the failed case.

Failed request:

qfqea

Successful request:

7zwr2

sethkinast commented 10 years ago

I cloned your repo (thanks for providing that) and am unable to reproduce the issue using an iPhone 5s and an iPhone 6. Is there a specific set of steps I should follow?

(Edit: I am using your "ioserror" branch that shows the login form like 10 times)

sixlettervariables commented 10 years ago

I've seen these sorts of failures with certain wireless setups before (odd QOS settings on routers, etc).

calvintwr commented 10 years ago

@sethkinast thanks for that.

yup the one that shows 10 login forms is correct. can you try the amazon EC2 instance hosted on http://vogue-verve.com:3000 you have to completely clear your history and browser cache.

@sixlettervariables the failures were reproduced on using different connection setups: i.e., various wireless connections, 3G, 4G mobile data. Both the localhost and an amazon EC2 instance reproduced the error.

sixlettervariables commented 10 years ago

Related: https://github.com/strongloop/express/issues/2406

calvintwr commented 10 years ago

@sixlettervariables thanks for that.

@aredridel @sethkinast I set up an nginx reverse proxy and the problem has totally gone away now.

aredridel commented 10 years ago

Oh now this is just interesting!

calvintwr commented 10 years ago

ever faced something like this in kraken?

aredridel commented 10 years ago

Not that I'm aware of, but it's specific enough that I don't know that I'd spot it.

dougwilson commented 10 years ago

The bug is being tracked here: https://github.com/jshttp/on-finished/issues/10 and a solution is forth-coming as soon as I have a working test for it :)

aredridel commented 10 years ago

Yay!

sethkinast commented 10 years ago

That issue has been closed. Is there anything we need to do on the Dust side, @dougwilson ?

dougwilson commented 10 years ago

So I was just looking around to even figure out what dust does :) So as far as I can tell, dust doesn't even have npm dependencies, is that right? In that case, the answer is nothing :) because it affected people who choose to integrate dust with express, right?

sethkinast commented 10 years ago

Yep! I wasn't sure if we needed to change our Stream implementation or anything or if it was solely outside of Dust's bailiwick. But we have no npm dependencies, so sounds we're good to go :) Thanks for looking!