fix(browser-pool): improve error handling when browser is not found #2050
Conversation
…s if browser fails to launch on Apify platform
| private _notifyOfFailedLaunch() { | ||
| let debugMessage = `Failed to launch browser. ${this.launchOptions?.executablePath ? 'Check whether the provided executable path is correct.' : ''} `; | ||
| if (process.env.APIFY_IS_AT_HOME) { | ||
| debugMessage += 'Make sure you used the correct Actor template for Playwright: ' |
There was a problem hiding this comment.
its about using correct docker image, not actor template, i'd not mention the templates here at all
There was a problem hiding this comment.
I think some new users and those who are not familiar with docker might not make the connection between the wrong docker image and the fact that they can fix the issue by instantiating with a new template. Maybe we could change it from Make sure to Try instantiating with that template ...? But for now I've removed the mention
| @@ -74,7 +74,12 @@ export class PuppeteerPlugin extends BrowserPlugin< | |||
| } | |||
| } catch (error) { | |||
| await close(); | |||
|
|
|||
| if (process.env.APIFY_IS_AT_HOME) { | |||
There was a problem hiding this comment.
it feels weird that for playwright we handle this outside of apify platform too, but here we dont. FYI puppeteer now has a similar CLI for installing the browsers https://pptr.dev/browsers-api/
not sure if its already mentioned in the error produced by puppetter?
There was a problem hiding this comment.
Not really. But the bad thing about this command is that it installs executable into the folder where it was executed, and under a long directory hierarchy, like chromium/mac_arm/-xxxx.../../.../MacOS/Chromium, and you have to be aware of that executable location, to e.g. pass it into PUPPETEER_EXECUTABLE_PATH
There was a problem hiding this comment.
It mentions troubleshooting guide though most of the times
Yes, the first screenshot is how it runs locally, without docker, if an executable is provided that is missing (playwright): |
|
The first screenshot looks like we retry that request? I'd say this should cause a hard failure. We have the |
…cause` instead of just rethrowing
I've updated it to throw CriticalError in the Puppeteer and Playwright plugins (not in BrowserPlugin, so as not to make assumptions about error handling for other possible implementations). It's a bit unpleasant though, that the error logging happens 3 times at different levels in such cases, but it still looks fine I guess: |
|
Should now only print once, the last couple of commits disable logging of CriticalError where it is about to be rethrown anyway |
|
Ok, that looks much better, let's see if it won't break any of the E2E tests: https://github.com/apify/crawlee/actions/runs/6026358796 |
B4nan
changed the title
Basically, just add a hint to the user that they might've forgotten to rebase their docker image/used the wrong template.
Closes #1459