Merge pull request #374 from fs-webdev/betterErrorLogging

better error handling/logging for failed builds

Author: joeycozza

CHANGELOG-FRONTIER.md

@@ -1,3 +1,9 @@
+## 8.8.1
+
+- Wrap the index-revision-replace code in a try/catch. Log the error message in the catch, but DON'T throw the error.
+  - By not throwing the error in the plugin, the already existing error logging will log the underlying issues as expected
+- add, (and commented out) a tap into afterCompile hook to check if errors exist. If so, then log those full error objects
+
 ## 8.8.0
 
 - Removed some proxies that are no longer needed, and scoped some others tighter to not conflict with localhost URLs

packages/react-scripts/config/index-revision-replace.js

@@ -1,27 +1,34 @@
-"use strict"
+'use strict'
 
 const fs = require('fs')
 const path = require('path')
-const { createHash } = require('crypto');
+const { createHash } = require('crypto')
 
 class IndexRevision {
-
   apply(compiler) {
     compiler.hooks.done.tap('IndexRevisionReplace', () => {
       const builtServiceWorkerPath = path.resolve('build/service-worker.js')
       const _indexHtmlPath = path.resolve('build/_index.html')
 
-      const _indexSrcCode = fs.readFileSync(_indexHtmlPath, 'utf-8')
-      const _indexHashRaw = md5(_indexSrcCode)
-      const _indexHashAsString = `"${_indexHashRaw}"`
-      
-      let serviceWorkerCode = fs.readFileSync(builtServiceWorkerPath, 'utf-8')
-      serviceWorkerCode = serviceWorkerCode.replace('self._INDEX_HASH', _indexHashAsString)
-
-      fs.writeFileSync(builtServiceWorkerPath, serviceWorkerCode)
+      try {
+        const _indexSrcCode = fs.readFileSync(_indexHtmlPath, 'utf-8')
+        const _indexHashRaw = md5(_indexSrcCode)
+        const _indexHashAsString = `"${_indexHashRaw}"`
+
+        let serviceWorkerCode = fs.readFileSync(builtServiceWorkerPath, 'utf-8')
+        serviceWorkerCode = serviceWorkerCode.replace('self._INDEX_HASH', _indexHashAsString)
+
+        fs.writeFileSync(builtServiceWorkerPath, serviceWorkerCode)
+      } catch (error) {
+        console.log(
+          `Failure in index-revision-replace.js. Almost always this is because of another failure in compilation. 
+Look for and fix other errors before addressing _index.html not found:`,
+          error.message,
+          '\n'
+        )
+      }
     })
   }
-
 }
 
 function md5(input) {

packages/react-scripts/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@fs/react-scripts",
-  "version": "8.8.0",
+  "version": "8.8.1",
   "upstreamVersion": "5.0.1",
   "description": "Configuration and scripts for Create React App.",
   "repository": {

packages/react-scripts/scripts/build.js

@@ -6,178 +6,183 @@
  * LICENSE file in the root directory of this source tree.
  */
 // @remove-on-eject-end
-'use strict';
+'use strict'
 
 // Do this as the first thing so that any code reading it knows the right env.
-process.env.BABEL_ENV = 'production';
-process.env.NODE_ENV = 'production';
+process.env.BABEL_ENV = 'production'
+process.env.NODE_ENV = 'production'
 
 // Makes the script crash on unhandled rejections instead of silently
 // ignoring them. In the future, promise rejections that are not handled will
 // terminate the Node.js process with a non-zero exit code.
 process.on('unhandledRejection', err => {
-  throw err;
-});
+  throw err
+})
 
 // Ensure environment variables are read.
-require('../config/env');
-
-const path = require('path');
-const chalk = require('react-dev-utils/chalk');
-const fs = require('fs-extra');
-const bfj = require('bfj');
-const webpack = require('webpack');
-const configFactory = require('../config/webpack.config');
-const paths = require('../config/paths');
-const checkRequiredFiles = require('react-dev-utils/checkRequiredFiles');
-const formatWebpackMessages = require('react-dev-utils/formatWebpackMessages');
-const printHostingInstructions = require('react-dev-utils/printHostingInstructions');
-const FileSizeReporter = require('react-dev-utils/FileSizeReporter');
-const printBuildError = require('react-dev-utils/printBuildError');
+require('../config/env')
+
+const path = require('path')
+const chalk = require('react-dev-utils/chalk')
+const fs = require('fs-extra')
+const bfj = require('bfj')
+const webpack = require('webpack')
+const configFactory = require('../config/webpack.config')
+const paths = require('../config/paths')
+const checkRequiredFiles = require('react-dev-utils/checkRequiredFiles')
+const formatWebpackMessages = require('react-dev-utils/formatWebpackMessages')
+const printHostingInstructions = require('react-dev-utils/printHostingInstructions')
+const FileSizeReporter = require('react-dev-utils/FileSizeReporter')
+const printBuildError = require('react-dev-utils/printBuildError')
 // frontier
 // coalesce per-locale locales for speedier intl perf scaling
-const { coalesceLocales } = require('./coalesceLocales');
+const { coalesceLocales } = require('./coalesceLocales')
 // /frontier
 
-const measureFileSizesBeforeBuild = FileSizeReporter.measureFileSizesBeforeBuild;
-const printFileSizesAfterBuild = FileSizeReporter.printFileSizesAfterBuild;
-const useYarn = fs.existsSync(paths.yarnLockFile);
+const measureFileSizesBeforeBuild = FileSizeReporter.measureFileSizesBeforeBuild
+const printFileSizesAfterBuild = FileSizeReporter.printFileSizesAfterBuild
+const useYarn = fs.existsSync(paths.yarnLockFile)
 
 // These sizes are pretty large. We'll warn for bundles exceeding them.
-const WARN_AFTER_BUNDLE_GZIP_SIZE = 512 * 1024;
-const WARN_AFTER_CHUNK_GZIP_SIZE = 1024 * 1024;
+const WARN_AFTER_BUNDLE_GZIP_SIZE = 512 * 1024
+const WARN_AFTER_CHUNK_GZIP_SIZE = 1024 * 1024
 
-const isInteractive = process.stdout.isTTY;
+const isInteractive = process.stdout.isTTY
 
 // Warn and crash if required files are missing
 if (!checkRequiredFiles([paths.appHtml, paths.appIndexJs])) {
-  process.exit(1);
+  process.exit(1)
 }
 
-const argv = process.argv.slice(2);
-const writeStatsJson = argv.indexOf('--stats') !== -1;
+const argv = process.argv.slice(2)
+const writeStatsJson = argv.indexOf('--stats') !== -1
 
 // Generate configuration
-const config = configFactory('production');
+const config = configFactory('production')
 
 // We require that you explicitly set browsers and do not fall back to
 // browserslist defaults.
-const { checkBrowsers } = require('react-dev-utils/browsersHelper');
+const { checkBrowsers } = require('react-dev-utils/browsersHelper')
 checkBrowsers(paths.appPath, isInteractive)
   .then(() => {
     // First, read the current file sizes in build directory.
     // This lets us display how much they changed later.
-    return measureFileSizesBeforeBuild(paths.appBuild);
+    return measureFileSizesBeforeBuild(paths.appBuild)
   })
   .then(previousFileSizes => {
     // Remove all content but keep the directory so that
     // if you're in it, you don't end up in Trash
-    fs.emptyDirSync(paths.appBuild);
+    fs.emptyDirSync(paths.appBuild)
     // Merge with the public folder
-    copyPublicFolder();
+    copyPublicFolder()
     // Start the webpack build
-    return build(previousFileSizes);
+    return build(previousFileSizes)
   })
   .then(
     ({ stats, previousFileSizes, warnings }) => {
       if (warnings.length) {
-        console.log(chalk.yellow('Compiled with warnings.\n'));
-        console.log(warnings.join('\n\n'));
+        console.log(chalk.yellow('Compiled with warnings.\n'))
+        console.log(warnings.join('\n\n'))
         console.log(
-          '\nSearch for the ' +
-          chalk.underline(chalk.yellow('keywords')) +
-          ' to learn more about each warning.'
-        );
-        console.log(
-          'To ignore, add ' + chalk.cyan('// eslint-disable-next-line') + ' to the line before.\n'
-        );
+          '\nSearch for the ' + chalk.underline(chalk.yellow('keywords')) + ' to learn more about each warning.'
+        )
+        console.log('To ignore, add ' + chalk.cyan('// eslint-disable-next-line') + ' to the line before.\n')
       } else {
-        console.log(chalk.green('Compiled successfully.\n'));
+        console.log(chalk.green('Compiled successfully.\n'))
       }
 
-      console.log('File sizes after gzip:\n');
+      console.log('File sizes after gzip:\n')
       printFileSizesAfterBuild(
         stats,
         previousFileSizes,
         paths.appBuild,
         WARN_AFTER_BUNDLE_GZIP_SIZE,
         WARN_AFTER_CHUNK_GZIP_SIZE
-      );
-      console.log();
-
-      const appPackage = require(paths.appPackageJson);
-      const publicUrl = paths.publicUrlOrPath;
-      const publicPath = config.output.publicPath;
-      const buildFolder = path.relative(process.cwd(), paths.appBuild);
-      printHostingInstructions(appPackage, publicUrl, publicPath, buildFolder, useYarn);
+      )
+      console.log()
+
+      const appPackage = require(paths.appPackageJson)
+      const publicUrl = paths.publicUrlOrPath
+      const publicPath = config.output.publicPath
+      const buildFolder = path.relative(process.cwd(), paths.appBuild)
+      printHostingInstructions(appPackage, publicUrl, publicPath, buildFolder, useYarn)
     },
     err => {
-      const tscCompileOnError = process.env.TSC_COMPILE_ON_ERROR === 'true';
+      const tscCompileOnError = process.env.TSC_COMPILE_ON_ERROR === 'true'
       if (tscCompileOnError) {
         console.log(
           chalk.yellow(
             'Compiled with the following type errors (you may want to check these before deploying your app):\n'
           )
-        );
-        printBuildError(err);
+        )
+        printBuildError(err)
       } else {
-        console.log(chalk.red('Failed to compile.\n'));
-        printBuildError(err);
-        process.exit(1);
+        console.log(chalk.red('Failed to compile.\n'))
+        printBuildError(err)
+        process.exit(1)
       }
     }
   )
   .catch(err => {
     if (err && err.message) {
-      console.log(err.message);
+      console.log(err.message)
     }
-    process.exit(1);
-  });
+    process.exit(1)
+  })
 
 // Create the production build and print the deployment instructions.
 function build(previousFileSizes) {
-  console.log('Creating an optimized production build...');
+  console.log('Creating an optimized production build...')
 
-  const compiler = webpack(config);
+  const compiler = webpack(config)
   return new Promise((resolve, reject) => {
     // frontier
     // coalesce per-locale locales for speedier intl perf scaling
     compiler.hooks.beforeRun.tap('perlocale', () => {
-      coalesceLocales(paths);
-    });
+      coalesceLocales(paths)
+    })
+
+    // UNCOMMENT FOR MORE DETAILS OF THE ERRORS IN A BUILD
+    // compiler.hooks.afterCompile.tap('afterCompile', compilation => {
+    //   if (compilation.errors && compilation.errors.length > 0) {
+    //     console.log(
+    //       chalk.red(
+    //         'Errors occurred during compilation. Try to resolve the problems, and reach out to the Frontier Core team if you need help:\n'
+    //       ),
+    //       compilation.errors
+    //     )
+    //   }
+    // })
+
     // /frontier
     compiler.run((err, stats) => {
-      let messages;
+      let messages
       if (err) {
         if (!err.message) {
-          return reject(err);
+          return reject(err)
         }
 
-        let errMessage = err.message;
+        let errMessage = err.message
 
         // Add additional information for postcss errors
         if (Object.prototype.hasOwnProperty.call(err, 'postcssNode')) {
-          errMessage +=
-            '\nCompileError: Begins at CSS selector ' +
-            err['postcssNode'].selector;
+          errMessage += '\nCompileError: Begins at CSS selector ' + err['postcssNode'].selector
         }
 
         messages = formatWebpackMessages({
           errors: [errMessage],
           warnings: [],
-        });
+        })
       } else {
-        messages = formatWebpackMessages(
-          stats.toJson({ all: false, warnings: true, errors: true })
-        );
+        messages = formatWebpackMessages(stats.toJson({ all: false, warnings: true, errors: true }))
       }
       if (messages.errors.length) {
         // Only keep the first error. Others are often indicative
         // of the same problem, but confuse the reader with noise.
         if (messages.errors.length > 1) {
-          messages.errors.length = 1;
+          messages.errors.length = 1
         }
-        return reject(new Error(messages.errors.join('\n\n')));
+        return reject(new Error(messages.errors.join('\n\n')))
       }
       if (
         process.env.CI &&
@@ -188,39 +193,39 @@ function build(previousFileSizes) {
         // Frontier Ignore eslint warnings as well
         const filteredWarnings = messages.warnings.filter(
           w => !/Failed to parse source map/.test(w) && !/\[eslint\]/.test(w)
-        );
+        )
         if (filteredWarnings.length) {
           console.log(
             chalk.yellow(
               '\nTreating warnings as errors because process.env.CI = true.\n' +
                 'Most CI servers set it automatically.\n'
             )
-          );
-          return reject(new Error(filteredWarnings.join('\n\n')));
+          )
+          return reject(new Error(filteredWarnings.join('\n\n')))
         }
       }
 
       const resolveArgs = {
         stats,
         previousFileSizes,
         warnings: messages.warnings,
-      };
+      }
 
       if (writeStatsJson) {
         return bfj
           .write(paths.appBuild + '/bundle-stats.json', stats.toJson())
           .then(() => resolve(resolveArgs))
-          .catch(error => reject(new Error(error)));
+          .catch(error => reject(new Error(error)))
       }
 
-      return resolve(resolveArgs);
-    });
-  });
+      return resolve(resolveArgs)
+    })
+  })
 }
 
 function copyPublicFolder() {
   fs.copySync(paths.appPublic, paths.appBuild, {
     dereference: true,
     filter: file => file !== paths.appHtml,
-  });
+  })
 }