-
Notifications
You must be signed in to change notification settings - Fork 22
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
Example HTTP/2 push does not appear to work #23
Comments
Thank you for the report. Was the step 4 your first connection to the server? In that case, pushes wouldn't happen because the auto-push plugin hadn't seen any traffic to record the request patterns. If you connect using another browser (or after clearing your cookie because the browser would've cached the resources), you would see pushes happen. I'll try |
I tested the sample app and as you reported, the push doesn't happen. Investigating now. It could be due to the recent upgrade of |
@mcollina It looks like the
Did something change in the recent version of fastify? |
@mcollina Never mind. It was my local setup that was broken. Hooks are called fine. Sorry for the noise. |
Thanks for looking into this @jinwoo.
Yes, but I in step 5 I reload to watch the network panel (and to trigger a reload, as I guessed auto-push plugin needed to record some request patterns). Seems like you figured this out by trying the example yourself, but figured I'd echo for clarity. |
Some things I noticed while debugging:
|
I could catch that error from |
@jinwoo I think it's a bug in Chrome/Chromium. See https://bugs.chromium.org/p/chromium/issues/detail?id=821492. @tsnieman can you check with Firefox too? |
If you generate your certificates with https://www.npmjs.com/package/tls-keygen, it should work correctly. @jinwoo we should update the blog post to use that tool. |
Thanks for the response @mcollina.
|
Yeah, using @tsnieman, I'm not sure why $ npx tls-keygen generated There's still the crash from |
I also forgot to mention that I had to update the |
Fixes google/node-fastify-auto-push#23. When recording the request patterns, don't use a path as a push key when it is also included in a push list for another key. It may cause a chain effect, causing unnecessary pushes. And it seems to crash the server sometimes. And also attach an error handler to the push stream.
Fixes google/node-fastify-auto-push#23. When recording the request patterns, don't use a path as a push key when it is also included in a push list for another key. It may cause a chain effect, causing unnecessary pushes. And it seems to crash the server sometimes. And also attach an error handler to the push stream.
Reopening while I'm addressing some more things. |
Now new npm versions of @tsnieman Please try again with the new version and let us know the result. Thanks! |
@jinwoo I seem to be having issues getting this working on Ubuntu despite the provided fixes. Here's some steps:
At this point, I'm loading via a "valid/secure" cert, but still not getting any pushed assets after several reloads: Though I will note that
I'm imagining that some of the issues I'm experiencing is because of Ubuntu/NSS versus Mac/Keychain: Is there a maintainer of this project using Ubuntu/Linux that can try using the example project? |
@tsnieman You have to clear the cookies and browser caches when you reload the page after the first load. Look at the HTTP response code, 304. It's because the browser already has those resources in its cache, it'll just reuse them. Cookies should also be cleared because auto-push stores the information related to what might be cached in the browser. |
@jinwoo Even after clearing cookies (or using something like a fresh incognito session), Chrome v66.0.3359.139 does not indicate "Push" -- only "Parser". |
@tsnieman I'm not sure what you mean by "Parser". What is the HTTP response code? Is it still 304? It means that the resources are still cached in the browser. You should clear both caches and cookies before reloading. Or use a different browser than the one used for the first load. |
"Parser" is referring to the initiator, per Chrome devtools network panel. A successful HTTP/2 Push should look like: I don't see "Push" in the "Initiator" column -- even using a fresh browser with cleared cache/cookies, or a fresh incognito session. This means that Chrome is loading assets after having parsed which assets to load from the HTML rather than having the assets in the browser's cache from the "Push". Just for clarity: I do this is with clear cache/cookies/browser after reloading several times in different browser. Yes, my screenshots show that assets are 304, but Chrome never indicates or indicated that any asset was ever pushed, so the 304 seems to be referring to a cache from a parsed asset. @jinwoo Would you please confirm that you are seeing "Push" in the initiator column of Chrome's devtools network panel? If there's anything else I can do to further debug why Chrome does not show "Push" in my initiator column -- even if it means I have to do a screen recording of this entire process -- please let me know. |
I do see "Push" in the devtools. If you see 304, the browser still has those resources in the browser cache. I'm not sure how you cleared them. |
@jinwoo OK, I think I got this workin. Seems to have just be a PEBCAK 🤤 Not really sure where I went wrong before (did the same thing I've been doing: testing in diff browsers, clearing caches, using "incognito mode", etc), but finally managed to get "Push" to show up reliably in incognito. Thank you for your time, and pardon the hassle. |
No problem. Glad that it works for you. And thanks again for reporting this issue so that we could fix! |
Basic process:
git clone git@github.com:google/node-fastify-auto-push.git
cd node-fastify-auto-push/samples/static-page
npm start -- --h2 --ap -p 3000
https://localhost:3000/
, bypass security warning, see "Wikipedia" load.nghttp2-client
as I have seen suggested for testing "Push" (wherein pushed assets are represented by an asterisk next to their asset row):nghttp
command seems to immediately crash our example server after displaying the above output. The crash is noted logged like this:Other details:
nvm
)node --expose-http2
flagPlease let me know if there's any additional information I should provide.
I deeply appreciate any time someone might spend thinking about this.
Thank you.
The text was updated successfully, but these errors were encountered: