lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Temp files not cleaned up in WSL

Open chrisrrowland opened this issue 1 year ago • 7 comments

FAQ

  • [X] Yes, my issue is not about variability or throttling.
  • [X] Yes, my issue is not about a specific accessibility audit (file with axe-core instead).

URL

https://www.google.com

What happened?

In WSL I am running this code like

node LighthouseScanner.mjs https://www.google.com

After it completes there is a temp folder left behind that looks like a full windows path.

lighthouse ls -l total 20 drwx------ 3 rowc rowc 4096 Nov 14 15:36 'C:\Users\chris\AppData\Local\lighthouse.42200815'

I don't see anything indicating an error.

import lighthouse from 'lighthouse';
import * as chromeLauncher from 'chrome-launcher';

(async () => {
  const chrome = await chromeLauncher.launch({
    chromeFlags: ['--headless', '--quiet'],
    chromePath: '/usr/bin/google-chrome-stable',
  });

  if (process.argv.length < 3) {
    console.error('Please provide a URL as a command line argument.');
    process.exit(1);
  }

  const url = process.argv[2];
  const options = {
    logLevel: 'info',
    output: ['html', 'json'],
    port: chrome.port,
    tmpdir: '/tmp/',
  };
  const runnerResult = await lighthouse(url, options);

  const combinedResult = {
    json: runnerResult.lhr,
    html: runnerResult.report,
  };

  // Output the combined result as a JSON object to stdout
  console.log(JSON.stringify(combinedResult, null, 2));

  await chrome.kill();
})();

What did you expect?

No temp files left behind.

What have you tried?

I have run it in directories not named lighthouse and the file name is still the same.

How were you running Lighthouse?

node

Lighthouse Version

11.2.0

Chrome Version

Google Chrome 118.0.5993.117

Node Version

v20.9.0

OS

WSL

Relevant log output

No response

chrisrrowland avatar Nov 14 '23 20:11 chrisrrowland

This is due to chrome-launcher.

  1. https://github.com/GoogleChrome/chrome-launcher/blob/main/src/utils.ts has os-specific functions for makeTmpDir. Can we just use os.tmpdir? This is...overly complicated and there's no comments justifying doing this manually. If we use os.tmpdir it wouldn't matter as much that we do not clear them on early terminations of the process.

  2. Should we do [kill()](https://github.com/GoogleChrome/chrome-launcher/blob/main/src/chrome-launcher.ts#L399C3-L399C8) in a nodejs exit handler?

connorjclark avatar Nov 14 '23 23:11 connorjclark

This is due to chrome-launcher.

That's interesting, I opened an issue over there a couple of weeks ago, but I assumed the temp file names meant this particular bit was a lighthouse issue.

chrisrrowland avatar Nov 14 '23 23:11 chrisrrowland

yeah I'm sure through a series of copy/paste we just left that in when extracting chrome-launcher out of lighthouse forever ago.

Small curiosity: this folder only has "lighthouse" in the name for win32/unix, not when under WSL, if I'm reading util.ts correctly. Since you are running on WSL, but still get this "lighthouse" named temp folder, I suppose our getPlatform() / isWSL code isn't working as we expected. I've never used WSL myself, and I believe this was 100% an external contribution, so I'll install WSL and play with it a bit to see what's going on.

@paulirish do you know why chrome-launcher does so much work to create a temp dir in a specific place, rather than just use os.tmpdir? Like, for WSL it tried to grep a user name from PATH to create a folder in that user's appdata, which just seems kinda intense. What's the importance of making these folders in a specific place? seems any ol' tmpdir given by the OS would suffice.

I'll give it a shot on my windows machine.

connorjclark avatar Nov 15 '23 00:11 connorjclark

@paulirish do you know why chrome-launcher does so much work to create a temp dir in a specific place,

no idea.

paulirish avatar Nov 15 '23 01:11 paulirish

I suppose our getPlatform() / isWSL code isn't working as we expected.

I'm pretty sure isWsl is correctly identifying my environment as WSL but I don't think the regex is working correctly. I had thrown out the idea of a parameter to specify a temp dir but os.tmpdir seems like it makes more sense.

Thanks for your attention on the matter!

chrisrrowland avatar Nov 15 '23 13:11 chrisrrowland

I'm getting even weirder paths after running lighthouse. ls -lAh:

drwx------   3 rudie rudie 4.0K Jan 16 09:36 '\\wsl.localhost\Ubuntu\var\www\dip\undefined\Users\undefined\AppData\Local\lighthouse.98673163'/

I'm in /var/www/dip and that's where I ran node node_modules/lighthouse/cli/index.js.

What are the 2 undefineds? With a special unicode (?) character? And a Windows username?

I have [email protected] and [email protected]. It does work well, I didn't even expect that. Well done WSL and Lighthouse 👍

rudiedirkx avatar Jan 16 '24 22:01 rudiedirkx

If I change getPlatform() to just return process.platform and never 'wsl', everything works perfectly. Maybe this is a WSL 1 vs 2 thing? Maybe wsl specific code was necessary in WSL 1, but that's what 'breaks' WSL 2, which is 'real' linux.

rudiedirkx avatar Mar 23 '24 13:03 rudiedirkx