clients(lr): increase Page.getAppManifest timeout to 10s

[connorjclark]

This was the issue behind /offline-ready?slow failing in Smokerider with service-worker.score = 0.

The smoketest mimics a SW that takes 5s to register by delaying the request for the .js by 5s*. Outgoing connections trip up LR w/ re: to fetching the manifest, as we saw before with long-polling, and this delay was causing the manifest fetch to fail.

By setting this timeout to x seconds, we are effectively saying "in LR, allow requests to take up to x seconds before giving up on the manifest". We don't want x to be too large, or we will get nothing (LR timeout). x shouldn't be too small either, but I'm not sure how to evaluate what it should be. 10 seconds seems closer than 3 seconds for a threshold where we start giving up on things.

It's unclear if any long request can cause this - GatherRunner.getWebAppManifest runs after the first pass, so perhaps all requests that the page makes must finish first? Would really like to understand this better.

* Sidenote: maybe the smoketest should have the SW do something that takes 5s instead? or is it not possible to delay registration like that?

Googlers can see: b/128446845#comment2

[patrickhulce]

maybe the smoketest should have the SW do something that takes 5s instead? or is it not possible to delay registration like that?

The original goal of that smoketest was to ensure that all the user has to do is request the service worker and we'll definitely wait for it to finish and recognize it. It sounds like it does the waiting on the next pass instead of between the pass/beforePass of the first load though which is why we're hitting getAppManifest while it's still loading. An alternate fix that would be good for LH overall is explicitly waiting on the service worker to finish up before completing the pass step. That's pretty involved though, so I'm on board for this as a band-aid :)

Sidenote: is there any effort to fix the root bug in LR?

[connorjclark]

Sidenote: is there any effort to fix the root bug in LR?

back when first seen w/ long-polling I filed a ticket with a (complicated) repro and gave it to the necessary developer. The effects are pretty minor and the bandaid simple enough, so I'm hesitant to elevate the issue. At the least, I'll cook up a much simpler repro from what I learned here, and bump the bug ticket.

[brendankenny]

The effects are pretty minor and the bandaid simple enough, so I'm hesitant to elevate the issue.

is this in reference to the smoke test or lengthening the timeout? It does seem to expose a real issue of what happens if a user's server is slow to return the manifest (or any file?) for some reason.

[connorjclark]

The effects are pretty minor and the bandaid simple enough, so I'm hesitant to elevate the issue.

is this in reference to the smoke test or lengthening the timeout? It does seem to expose a real issue of what happens if a user's server is slow to return the manifest (or any file?) for some reason.

in reference to WRS blocking on requesting the manifest, when other requests are pending

[paulirish]

At the very least we need to file a bug on WRS for this odd manifest fetching behavior.

Hypothetically there could be other cases where fetches are waiting for pending requests. (like maybe the sw.js request is.)

Let's file a b/ bug, give to caseq and link up the bug in the comment here.

[connorjclark]

b/124008171

…

On Wed, Apr 17, 2019, 1:53 PM Paul Irish ***@***.***> wrote: At the very least we need to file a bug on WRS for this odd manifest fetching behavior. Hypothetically there could be other cases where fetches are waiting for pending requests. (like maybe the sw.js request is.) Let's file a b/ bug, give to caseq and link up the bug in the comment here. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#8350 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7CAMXW62URBLCPMA5QIALPQ6E5ZANCNFSM4HGQ2CQA> .

[paulirish]

For those that can't see the bug...

I'll have to tell you that @hoten already did exactly what I said... 2½ months ago. 🤣

[connorjclark]

I anticipated this very moment 8)

I'll work out a simpler repro tomorrow and send it over to caseq

[patrickhulce]

WFM! 🕙

[connorjclark]

@paulirish all the important bits are green