link extraction promise cleanup: #701
Conversation
ikreymer
commented
Oct 10, 2024
ikreymer
commented
- catch frame.evaluate() directly and log errors there to avoid any possibility of exception being propagated before wrapping in timedRun()
- also add clearTimeout() to timedRun()
- possibly fixes Puppeteer errors causing program termination openzim/zimit#376
…omise errors, then wrap in timedRun() possibly fixes openzim/zimit#376
| @@ -38,8 +40,8 @@ export function timedRun( | |||
| //logger.error("Unknown exception", {...errJSON(err), ...logDetails}, context); | |||
| throw err; | |||
There was a problem hiding this comment.
It looks like we have a few uses of timedRun that aren't wrapped in a tightly scoped try/catch block (e.g. setting page title in the crawlPage method, running behaviors) but rather a higher-level one in the worker that will catch and log a "Worker Exception". I'm wondering if we might want to log the error closer to where it occurs or append some contextual information to the error message to help debug when something goes wrong?
There was a problem hiding this comment.
Hm, i think that this should still catch the original error, but maybe worth another look.
This was removed to avoid redundancy error message - if the timedRun() throws, its not a timeout, but an actual exception that probably should interrupt the crawling of that page...
