search

foo-software / lighthouse-check-action

GitHub Action for running @GoogleChromeLabs Lighthouse audits with all the bells and whistles 🔔 Multiple audits, Slack notifications, and more!
https://github.com/marketplace/actions/lighthouse-check
MIT License
471 stars 24 forks source link

extraHeaders Set-Cookie not applyed #93

Closed schoenwaldnils closed 2 years ago

schoenwaldnils commented 2 years ago

This config:

extraHeaders: '{ "Set-Cookie": "USE_COOKIE_CONSENT_STATE={%22session%22:false}" }'

does not seem to be applyed. I'd exspect the USE_COOKIE_CONSENT_STATE cookie to be set and therfore the cookie banner not to be visible. But it doesn't work :/

schoenwaldnils commented 2 years ago

Nevermind .. this way it works

extraHeaders: '{ "Set-Cookie": "USE_COOKIE_CONSENT_STATE={%22session%22:false}; Path=/" }'
schoenwaldnils commented 2 years ago

Nevermind again :/ it seems the one test with the path just was a fluke. Now the cookie doesn't seem to be set and the cookie banner is shown 🤔

adamhenson commented 2 years ago

@schoenwaldnils - Set-Cookie is a response header. You cannot set it in the request and expect it to forward to the response. You need to figure what the request header is that will trigger your server to respond with that response header.

adamhenson commented 2 years ago

In summary:

I just dug through this project again and re-kicked all tests. The extraHeaders feature is tested in this project and confirmed that it still works as expected. Here's how it's tested:

  1. A simple Vercel Express app is deployed with every MR. It renders cookies in the HTML.
  2. In CI we run the latest GitHub Action code against the corresponding URL above with extraHeaders set.
  3. From the above we can confirm in the Lighthouse report screenshot that the headers exist as they were rendered in the HTML, thanks to our app established in step 1 (the Lighthouse report URL is output in CI logs... screenshot below of the most recent report).
Screen Shot 2022-05-20 at 9 24 37 AM
adamhenson commented 2 years ago

@schoenwaldnils as I just spent a half hour of my day on this... please next time do your due diligence and experiment with your implementation before opening an issue in an open source project.

Contributors of open source projects volunteer time to maintain, and it's not fair for us to spend time on something that the issue opener has barely spent on. Good luck.

schoenwaldnils commented 2 years ago

@adamhenson I just duplicated your headerServer.ts on my project and can confirm that the headers are set. You are correct with Set-Cookie being a response header. I wrongly assumed they would work in the extraHeaders.

I don't open issues before testing it on my end. On this I spend at least 2h on trying to figure out the problem. But sometimes I get stuck and just can't figure out whats wrong.

Be sure I did't wanted to waste you time

adamhenson commented 2 years ago

I get it, thanks. Good luck.