magesuite / magepack

Next generation Magento 2 advanced JavaScript bundler.
Open Software License 3.0
438 stars 91 forks source link

Return early when an HTTP error is detected #143

Closed fredden closed 2 years ago

fredden commented 2 years ago

Fixes #71

Before:

ℹ Collecting bundle modules in the browser.
ℹ Collecting modules for bundle "category".

 ERROR  Navigation timeout of 30000 ms exceeded

  at magepack/node_modules/puppeteer/lib/LifecycleWatcher.js:142:21
  at async FrameManager.navigateFrame (magepack/node_modules/puppeteer/lib/FrameManager.js:94:17)
  at async Frame.goto (magepack/node_modules/puppeteer/lib/FrameManager.js:406:12)
  at async Page.goto (magepack/node_modules/puppeteer/lib/Page.js:672:12)
  at async Object.category (magepack/lib/generate/collector/category.js:36:5)
  at async module.exports (magepack/lib/generate.js:70:13)
  -- ASYNC --
  at Frame.<anonymous> (magepack/node_modules/puppeteer/lib/helper.js:111:15)
  at Page.goto (magepack/node_modules/puppeteer/lib/Page.js:672:49)
  at Page.<anonymous> (magepack/node_modules/puppeteer/lib/helper.js:112:23)
  at Object.category (magepack/lib/generate/collector/category.js:36:16)
  at processTicksAndRejections (node:internal/process/task_queues:96:5)
  at async module.exports (magepack/lib/generate.js:70:13)

After:

ℹ Checking URLs...
ℹ Error returned while testing https://www.example.test/

 ERROR  unable to verify the first certificate

  at TLSSocket.onConnectSecure (node:_tls_wrap:1530:34)
  at TLSSocket.emit (node:events:390:28)
  at TLSSocket._finishInit (node:_tls_wrap:944:8)
  at TLSWrap.ssl.onhandshakedone (node:_tls_wrap:725:12)
fredden commented 2 years ago

@krzksz yes, that looks like a good approach. I will open a separate pull request to add error detection via Puppeteer.

Part of the reasoning for doing this separately was to get a good error message out. The testing I've done today shows that we already get good-enough error messages from Puppeteer when there are common connection issues (like DNS failure / domain not found, or TCP connection refused). Adding detection of non-200 response codes should complete the picture.

The other bonus about doing this early was that we get faster feedback for users. Requesting the page allows Magento to load its caches before the Puppeteer time-outs start, and we can test all the URLs at the start in parallel before trying to collect modules from these in sequence.

It also looks like Puppeteer is ignoring certificate issues, so the example given in the pull request description isn't helpful.