2
\$\begingroup\$

I am using puppeteer to extract data and see when a car that meets my requirements shows up, this is what I did so far. I would like some basic syntax advice, or more advanced tips as well.

I tried to follow this best practices:

  • Give meaningful names to variables
  • Only explain what seems not obvious
  • Split code into functions to make it more readable.

Currently the code takes a while, I am not sure how to speed it up just yet.

Code


import puppeteer, {type Page, type Browser} from "puppeteer";

(async () => {
  const browser = await puppeteer.launch();
  const page = await browser.newPage();
  const baseURL = "https://www.arnoldclark.com/used-cars";
  //based off conditions that I want, done manually in the website
  const city = "cityName"
  const search =
    "?payment_type=cash&min_price=4000&max_price=8000&location=${city}%2C UK&photos_only=true&unreserved_only=true&roadtax_cost=30&mileage=72000&age=6&seats[]=4&seats[]=5&show_click_and_collect_options=true&sort_order=mileage";

  await page.goto(baseURL + search);

  // car URL from link
  const carUrls = await page.$$eval("h2.ac-vehicle__title a", (carLinks) =>
    carLinks.map((link) => link.href)
  );

  const candidates = await getCandidates(browser, carUrls);

  console.table(candidates);

  await browser.close();
})();

interface Car { 
  favs: number;
  url: string;
}

async function getCandidates(browser: Browser, carUrls: string[]) {
  const candidates:Car[] = [];
  await Promise.all(
    carUrls.map(async (url) => {
      const newPage = await browser.newPage();
      await newPage.goto(url);
      try {
        // may be loaded dynamically
        const favs = await newPage.$eval(
          ".ac-productheader__content-wrapper span",
          (el) => el.textContent
        );
        if(favs) candidates.push({ favs: Number.parseInt(favs.split(" ")[0]), url });
      } catch (e) {
        console.log("no favs");
      }
      newPage.close()
    })
  );
  return candidates.sort((a, b) => b.favs - a.favs).slice(0, 4);
}

Sample output

┌─────────┬──────┬───────────────────────────────────────────────────────────────────────────────────────────────────┐
│ (index) │ favs │                                                url                                                │
├─────────┼──────┼───────────────────────────────────────────────────────────────────────────────────────────────────┤
│    0    │  87  │       'https://www.arnoldclark.com/used-cars/kia/picanto/1-0-1-5dr/2018/ref/arnai-u-92166'        │
│    1    │  31  │     'https://www.arnoldclark.com/used-cars/suzuki/celerio/1-0-sz3-5dr/2018/ref/arnai-u-92266'     │
│    2    │  31  │       'https://www.arnoldclark.com/used-cars/ford/ka/1-2-studio-5dr/2017/ref/arnaj-u-41340'       │
│    3    │  29  │ 'https://www.arnoldclark.com/used-cars/dacia/sandero/1-2-16v-ambiance-5dr/2017/ref/arndv-u-75527' │
└─────────┴──────┴───────────────────────────────────────────────────────────────────────────────────────────────────┘
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Did you checked times to run parts? Like if the request was long, if it's because of sorting etc? \$\endgroup\$
    – Elikill58
    Commented Apr 20, 2023 at 7:15
  • \$\begingroup\$ wdym ? I think it just takes time because it is http-ing to each URL @Elikill58 \$\endgroup\$
    – Mah Neh
    Commented Apr 20, 2023 at 10:21
  • 1
    \$\begingroup\$ Yes but how much? Does one URL is more longer than others? Are they representing 90% of total time or 10% of it? It can be a good start to know where time is used \$\endgroup\$
    – Elikill58
    Commented Apr 20, 2023 at 11:01
  • \$\begingroup\$ Using ({headless:false}) 99% of the time is occupied by the process sending 20 http requests for pages (and each page requests extra assets) at once and waiting the content to arrive. @Elikill58 \$\endgroup\$
    – Mah Neh
    Commented Apr 20, 2023 at 12:13

1 Answer 1

3
\$\begingroup\$

Bug

The following string attempts to interpolate a variable:

const search =
    "?payment_type=...&location=${city}...";

This should be a template literal with backticks:

const search =
    `?payment_type=...&location=${city}...`;

Unused import

Page was imported but never used.

Autoformatting

Use Prettier so your formatting is standard and consistent.

Move cleanup calls to finally blocks

If an error occurs, the browser will never close and the process will hang. Put page.close() and browser.close() in finally blocks so they'll be cleaned up regardless of whether an error occurs or not.

Use page.waitForSelector()

There's a comment // may be loaded dynamically. If that's true, you probably want to wait for that element with page.waitForSelector().

Use domcontentloaded

await page.goto(baseURL + search); uses the default load event, which is slower than domcontentloaded since it waits for resources to load. Prefer await page.goto(baseURL + search, {waitUntil: "domcontentloaded"}); if you don't need to wait for anything but a particular element. See this blog post of mine for a more thorough explanation.

Don't use map for side effects

Your map is overloaded to filter as well, by mutating the candidates array, which is a bit confusing. If you expect some operations may fail and need to filter them out, Promise.allSettled is the more idiomatic tool than Promise.all when you expect some to fail. See Crawling multiple URLs in a loop using Puppeteer.

Be careful with Promise.all--if the parallelism exceeds your computer's resources, performance can suffer. Consider using a task queue if you expect more than a few links.

Generally, catch and error logging should be moved to the calling scope rather than left inside a function, which should have no side effects.

await all promises

newPage.close() should probably be awaited because Puppeteer tends to throw errors if there are interleaved operations. As above, use finally to ensure it runs in the presence of errors.

Block unnecessary resources

Since you're just scraping a bit of text, there's no need to wait for images, stylesheets, analytics scripts and so forth to load. Use page.on("request", request => {}) to abort any requests for resources you don't care about.

Use the browser's default page

The browser comes with a page already open. You can use it with const [page] = await browser.pages();.

Use parseInt(num, 10)

Be explicit about your radix to avoid gotchas. q

Break out more functions

For a small script, this isn't that important, but as the script grows it's a good idea to break out more functions as sensible cut points emerge.

For example, getCandidates could abstract away the "get one candidate" logic that's inside of map into a separate function. Also, scraping car URLs from the initial page could be a function to clean up the main IIFE a bit.

Suggested rewrite

The page has changed, so the selectors no longer exist, but I'd suggest something like this:

import puppeteer, { type Page, type Browser } from "puppeteer"; // ^22.7.1

interface Car {
  favs: number;
  url: string;
}

let browser;
(async () => {
  browser = await puppeteer.launch();
  const [page] = await browser.pages();
  const baseURL = "https://www.arnoldclark.com/used-cars";
  const city = "cityName";
  const search =
    `?payment_type=cash&min_price=4000&max_price=8000&location=${city}%2C UK&photos_only=true&unreserved_only=true&roadtax_cost=30&mileage=72000&age=6&seats[]=4&seats[]=5&show_click_and_collect_options=true&sort_order=mileage`;
  await setResourceBlocks(page);
  const carURLs = await getCarLinks(page, baseURL + search);
  const candidates = await getCandidates(browser, carURLs);
  console.table(candidates);
})()
  .catch((err: Error) => console.error(err))
  .finally(() => browser?.close());

async function setResourceBlocks(page: Page): Promise<void> {
  await page.setRequestInterception(true);
  const blockedResourceTypes = [
    "image", "stylesheet", "font", "object",
    "texttrack", "imageset", "beacon", "csp_report",
    "TODO just an example",
  ];
  const blockedURLs = [
    "TODO fill in",
  ];
  const allowedRequest = req =>
    !blockedResourceTypes.includes(req.resourceType()) &&
    !blockedURLs.includes(req.url());
  page.on("request", req => {
    if (allowedRequest(req)) {
      req.continue();
    } else {
      req.abort();
    }
  });
}

async function getCarLinks(page: Page, url: string): Promise<string[]> {
  await page.goto(url, { waitUntil: "domcontentloaded" });
  const selector = "h2.ac-vehicle__title a";
  await page.waitForSelector(selector);
  return page.$$eval(selector, carLinks =>
    carLinks.map(link => link.href)
  );
}

async function getCandidate(browser: Browser, url: string): Promise<Car> {
  let page;
  try {
    page = await browser.newPage();
    await page.goto(url, { waitUntil: "domcontentloaded" });
    const selector = ".ac-productheader__content-wrapper span";
    await page.waitForSelector(selector);
    const favs: number = await page.$eval(
      selector,
      el => Number.parseInt(el.textContent?.split(/\s+/)[0] || '', 10)
    );
    return {
      favs,
      url,
    };
  } finally {
    await page?.close();
  }
}

async function getCandidates(browser: Browser, carUrls: string[]): Promise<Car[]> {
  const candidates = await Promise.allSettled(
    carUrls.map(url => getCandidate(browser, url))
  );
  return candidates
    .filter(e => e.status === "fulfilled")
    .map(e => e.value)
    .sort((a, b) => b.favs - a.favs)
    .slice(0, 4);
}

It's a bit longer due to the resource block function and factoring the functions.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.