Surface runtime errors in Deploy Log and Deploy Summary #505

jackbrewer · 2022-10-26T13:44:49Z

Closes https://github.com/netlify/netlify-plugin-lighthouse/issues/503
Closes https://github.com/netlify/netlify-plugin-lighthouse/issues/494
Closes https://github.com/netlify/netlify-plugin-lighthouse/issues/415

Likely solves others with mention of undefined results. We should do an issue sweep on Thurs

Ensures any runtime errors reported by Lighthouse are passes around alongside the rest of the data. This is then surfaced in the Deploy Log (directly after the failing run) and as an item in the Deploy summary.

This PR also moves some "data formatting" functions to a standalone file and adds tests. In my first attempt, these were being more heavily modified to deal with the runtimeError, but I ended up undoing that. It seemed a shame to put the functions back, so I've kept them. I've marked the exact sections which changes for easier reviewing.

For Netlifiers, check out https://app.netlify.com/sites/with-lighthouse-plugin/deploys/6359368d902825000811cdfa#L695

Deploy Summary
Three runs, one of which failed:

We use the error string provided by Lighthouse. I considered writing a few shorter versions to go in the Deploy Summary but seeing the list of possible variations made me think again!

Deploy Log

We're showing the full error here, not just the message part. This should help add some google-ability using the code incase the message text changes in future.

jackbrewer · 2022-10-26T13:46:24Z

src/format.js

+  return actual < expected;
+};
+
+const getError = (id, expected, categories, audits) => {


Unchanged, just moved and tested

jackbrewer · 2022-10-26T13:46:54Z

src/format.js

+const { minify } = require('html-minifier');
+const { makeReplacements } = require('./replacements');
+
+const belowThreshold = (id, expected, categories) => {


Function unchanged, just moved and tested

jackbrewer · 2022-10-26T13:50:20Z

src/format.js

+  return { message: categoryError, details: categoryAudits };
+};
+
+const formatShortSummary = (categories) => {


Split out from following function

jackbrewer · 2022-10-26T13:50:28Z

src/format.js

+};
+
+const formatResults = ({ results, thresholds }) => {
+  const runtimeError = results.lhr.runtimeError;


jackbrewer · 2022-10-26T13:51:08Z

src/format.js

+    ...(thresholds[id] ? { threshold: thresholds[id] } : {}),
+  }));
+
+  const shortSummary = formatShortSummary(categories);


Now uses formatShortSummary (previous function in file) as a standalone function rather than doing in-place

jackbrewer · 2022-10-26T13:52:17Z

src/format.js

+    minifyJS: true,
+  });
+
+  return { summary, shortSummary, details, report, errors, runtimeError };


Adds runtimeError to the end

jackbrewer · 2022-10-26T13:53:43Z

src/index.js

+            }, {});
+          }
+
+          if (runtimeError) {


Send the runtimeError off to the Deploy Summary, instead of shortSumary (which is undefined)

jackbrewer · 2022-10-26T13:54:54Z

src/index.js

@@ -289,17 +240,23 @@ module.exports = {
            output_path,
            settings,
          });
-        if (summary) {
+
+        if (summary && !runtimeError) {


Don't show the summary when there's an error

jackbrewer · 2022-10-26T13:55:30Z

src/index.js


        const fullPath = [serveDir, path].join('/');
        if (report) {
          const size = Buffer.byteLength(JSON.stringify(report));
          console.log(
            `Report collected: audited_uri: '${chalk.magenta(
              url || fullPath,
-            )}', html_report_size: ${chalk.magenta(size / 1024)} KiB`,
+            )}', html_report_size: ${chalk.magenta(
+              +(size / 1024).toFixed(2),


Random bonus fix to round the file size to maximum 2 decimal places

jackbrewer · 2022-10-26T13:56:13Z

src/lighthouse.js

@@ -54,9 +54,6 @@ const runLighthouse = async (browserPath, url, settings) => {
      },
      settings,
    );
-    if (results.lhr.runtimeError) {


Needed to be removed to allow the error to be handle more gracefully elsewhere

jackbrewer · 2022-10-26T14:07:19Z

src/format.js

+  return actual < expected;
+};
+
+const getError = (id, expected, categories, audits) => {


Function unchanged, just moved and tested

aitchiss

Nice! This is going to make things much clearer ✨

aitchiss · 2022-10-27T08:33:34Z

src/index.js

 const persistResults = async ({ report, path }) => {
  await fs.mkdir(dirname(path), { recursive: true });
  await fs.writeFile(path, report);
 };

 const getUtils = ({ utils }) => {
+  // This function checks to see if we're running within the Netlify Build system,


Love the additional comments 😍

netlify · 2022-10-27T14:08:02Z

👷 Deploy Preview for plugin-lighthouse processing.

Name	Link
🔨 Latest commit	`f190323`
🔍 Latest deploy log	https://app.netlify.com/sites/plugin-lighthouse/deploys/635a9ec3cab6a400096d0783

jackbrewer · 2022-10-27T14:16:57Z

CI is failing on the coloured strings. I'll try some simpler partial text matching instead

jackbrewer · 2022-10-27T15:14:59Z

Managed to get there eventually… tests are passing on CI now that ANSI codes are stripped

jackbrewer added 2 commits October 26, 2022 13:03

Move formatting function to new file, pass error to shortSummary

3225e8c

keep runtimeError out of shortSummary

1109cb3

jackbrewer commented Oct 26, 2022

View reviewed changes

jackbrewer requested a review from aitchiss October 26, 2022 13:59

jackbrewer self-assigned this Oct 26, 2022

jackbrewer commented Oct 26, 2022

View reviewed changes

aitchiss previously approved these changes Oct 27, 2022

View reviewed changes

jackbrewer marked this pull request as ready for review October 27, 2022 14:06

jackbrewer requested a review from a team October 27, 2022 14:06

Merge branch 'main' into jbr/surfaceErrorInSummary

56d24b0

Remove colours from strings during tests

f7d63d1

jackbrewer dismissed aitchiss’s stale review via f7d63d1 October 27, 2022 14:38

jackbrewer added 2 commits October 27, 2022 16:05

Now with added node 14 support

dbd8f9a

linting…

f190323

jackbrewer requested a review from aitchiss October 27, 2022 15:15

aitchiss approved these changes Oct 27, 2022

View reviewed changes

jackbrewer merged commit 77ccef3 into main Oct 27, 2022

jackbrewer deleted the jbr/surfaceErrorInSummary branch October 27, 2022 15:26

token-generator-app bot mentioned this pull request Oct 27, 2022

chore(main): release 4.0.2 #508

Merged

Surface runtime errors in Deploy Log and Deploy Summary #505

Surface runtime errors in Deploy Log and Deploy Summary #505

Uh oh!

Conversation

jackbrewer commented Oct 26, 2022 • edited by aitchiss Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jackbrewer Oct 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jackbrewer Oct 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aitchiss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

netlify bot commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

👷 Deploy Preview for plugin-lighthouse processing.

Uh oh!

jackbrewer commented Oct 27, 2022

Uh oh!

jackbrewer commented Oct 27, 2022

Uh oh!

Uh oh!

jackbrewer commented Oct 26, 2022 •

edited by aitchiss

Loading

jackbrewer Oct 26, 2022 •

edited

Loading

jackbrewer Oct 26, 2022 •

edited

Loading

netlify bot commented Oct 27, 2022 •

edited

Loading