Support sass errors in error pages

alphagov · Aug 4, 2023 · d93d726 · d93d726
1 parent 84cd3ee
commit d93d726
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 21 deletions.
diff --git a/lib/build.js b/lib/build.js
@@ -165,20 +165,15 @@ function _generateCssSync (sassPath, cssPath, options = {}) {
   fse.readdirSync(sassPath)
     .filter(file => ((file.endsWith('.scss') && !filesToSkip.includes(file))))
     .forEach(file => {
-      try {
-        const result = sass.compile(path.join(sassPath, file), {
-          quietDeps: true,
-          loadPaths: [projectDir],
-          sourceMap: true,
-          style: 'expanded'
-        })
-
-        const cssFilename = filesToRename[file] || file.replace('.scss', '.css')
-        fse.writeFileSync(path.join(cssPath, cssFilename), result.css)
-      } catch (err) {
-        console.error(err.message)
-        console.error(err.stack)
-      }
+      const result = sass.compile(path.join(sassPath, file), {
+        quietDeps: true,
+        loadPaths: [projectDir],
+        sourceMap: true,
+        style: 'expanded'
+      })
+
+      const cssFilename = filesToRename[file] || file.replace('.scss', '.css')
+      fse.writeFileSync(path.join(cssPath, cssFilename), result.css)
     })
 }
 

diff --git a/lib/dev-server.js b/lib/dev-server.js
@@ -31,15 +31,24 @@ const { logPerformanceSummaryOnce, startPerformanceTimer, endPerformanceTimer }
 // Build watch and serve
 async function runDevServer () {
   await collectDataUsage()
+  let startupError
 
-  generateAssetsSync()
+  try {
+    generateAssetsSync()
+  } catch (err) {
+    startupError = err
+  }
   await utils.waitUntilFileExists(path.join(publicCssDir, 'application.css'), 5000)
 
   const port = await (new Promise((resolve) => utils.findAvailablePort(resolve)))
 
   watchBeforeStarting()
   const nodemon = await runNodemon(port)
   watchAfterStarting(nodemon)
+
+  if (startupError) {
+    nodemon.emit('restart')
+  }
 }
 
 function proxyAndGenerateCssSync (fullFilename) {

diff --git a/lib/errorServer.js b/lib/errorServer.js
@@ -6,6 +6,7 @@ const { getErrorModel } = require('./utils/errorModel')
 const { verboseLog } = require('./utils/verboseLogger')
 const syncChanges = require('./sync-changes')
 const { flagError } = require('./sync-changes')
+const { packageDir } = require('./utils/paths')
 
 function runErrorServer (error) {
   flagError(error)
@@ -72,7 +73,7 @@ function runErrorServer (error) {
     try {
       const nunjucksAppEnv = getNunjucksAppEnv([
         path.join(__dirname, 'nunjucks'),
-        path.join(process.cwd(), 'node_modules', 'govuk-frontend')
+        path.join(packageDir, 'node_modules', 'govuk-frontend')
       ])
       res.end(nunjucksAppEnv.render('views/error-handling/server-error', getErrorModel(error)))
     } catch (ignoreThisError) {

diff --git a/lib/utils/errorModel.js b/lib/utils/errorModel.js
@@ -35,6 +35,23 @@ function getSourcecode (filePath, errorLineNumber, maxLines) {
   }
 }
 
+function sassMatcher (error) {
+  const matcher = /([a-zA-Z0-9\-@_/.]+)\s+(\d+):(\d+)\s+/
+  const matches = matcher.exec(error.message)
+  if (!matches) {
+    verboseLog('Not sassMatcher')
+    return
+  }
+  const [, filePath, line, column] = matches
+  return {
+    message: error.message.split('/n')[0],
+    filePath,
+    line: line && parseInt(line, 10),
+    column: column && parseInt(column, 10),
+    processedBy: 'sassMatcher'
+  }
+}
+
 function nunjucksMatcherWithLineAndColumnSpelledOut (error) {
   const matcher = /\(((?:\/|[A-Z]:\\)[^)]+)\) \[Line (\d+), Column (\d+)]\s+(.+)$/
   const matches = matcher.exec(error.message)
@@ -69,7 +86,7 @@ function nunjucksMatch (error) {
   return {
     message: processMessage(error.message),
     filePath,
-    processedBy: 'nunjucksMatcherA'
+    processedBy: 'nunjucksMatch'
   }
 }
 
@@ -134,6 +151,9 @@ function fallback (error) {
 }
 
 function parseError (error) {
+  if (sassMatcher(error)) {
+    return sassMatcher(error)
+  }
   if (nunjucksMatcherWithLineAndColumnSpelledOut(error)) {
     return nunjucksMatcherWithLineAndColumnSpelledOut(error)
   }

diff --git a/lib/utils/errorModel.spec.js b/lib/utils/errorModel.spec.js
@@ -139,7 +139,7 @@ describe('getErrorDetails', () => {
       message: 'template not found: layouts/main2.html',
       filePath: '/Users/example.user/prototype-kits/companion-prototype-kit/app/views/index.njk',
       normalisedFilePath: '/Users/example.user/prototype-kits/companion-prototype-kit/app/views/index.njk',
-      processedBy: 'nunjucksMatcherA'
+      processedBy: 'nunjucksMatch'
     })
   })
   it('should process a nunjucks error using a variable as a function', () => {
@@ -167,7 +167,7 @@ describe('getErrorDetails', () => {
       message: 'Unable to call `serviceName`, which is not a function',
       filePath: '/Users/example.user/prototype-kits/companion-prototype-kit/app/views/index.njk',
       normalisedFilePath: '/Users/example.user/prototype-kits/companion-prototype-kit/app/views/index.njk',
-      processedBy: 'nunjucksMatcherA'
+      processedBy: 'nunjucksMatch'
     })
   })
   it('should process a node console not function', () => {
@@ -308,7 +308,7 @@ describe('getErrorDetails', () => {
       message: 'Unable to call `serviceName`, which is not a function',
       filePath: 'X:\\Users\\example.user\\my-prototype\\app\\views\\index.njk',
       normalisedFilePath: 'app\\views\\index.njk',
-      processedBy: 'nunjucksMatcherA'
+      processedBy: 'nunjucksMatch'
     })
   })
 
@@ -333,7 +333,7 @@ describe('getErrorDetails', () => {
       message: 'template not found: layouts/main2.html',
       filePath: 'X:\\Users\\example.user\\my-prototype\\app\\views\\index.njk',
       normalisedFilePath: 'app\\views\\index.njk',
-      processedBy: 'nunjucksMatcherA'
+      processedBy: 'nunjucksMatch'
     })
   })
 
@@ -424,6 +424,40 @@ Require stack:\n- /Users/example.user/projects/prototype-kits/companion-prototyp
     })
   })
 
+  it('should process a sass syntax error', () => {
+    expect(getErrorDetails(unixCwd, {
+      message: `expected "{".
+[34m  ╷[0m
+[34m1 │[0m sdfsf[31m[0m
+[34m  │[0m [31m     ^[0m
+[34m  ╵[0m
+  app/assets/sass/cheese.scss 1:6  root stylesheet`,
+      stack: `Error: expected "{".
+[34m  ╷[0m
+[34m1 │[0m sdfsf[31m[0m
+[34m  │[0m [31m     ^[0m
+[34m  ╵[0m
+  app/assets/sass/cheese.scss 1:6  root stylesheet
+    at Object.wrapException (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:1247:17)
+    at SpanScanner.error$3$length$position (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:67896:15)
+    at SpanScanner.expectChar$2$name (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:67978:12)
+    at SpanScanner.expectChar$1 (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:67981:19)
+    at ScssParser0.children$1 (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:91335:10)
+    at ScssParser0._stylesheet0$_withChildren$1$3 (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:97629:39)
+    at ScssParser0._stylesheet0$_withChildren$3 (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:97634:19)
+    at ScssParser0._stylesheet0$_styleRule$2 (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:94688:20)
+    at ScssParser0._stylesheet0$_variableDeclarationOrStyleRule$0 (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:94539:22)
+    at ScssParser0._stylesheet0$_statement$1$root (/home/ben/WebstormProjects/govuk-prototype-kit/node_modules/sass/sass.dart.js:94462:215)`
+    })).toEqual({
+      message: 'expected "{".',
+      filePath: 'app/assets/sass/cheese.scss',
+      normalisedFilePath: 'app/assets/sass/cheese.scss',
+      line: 1,
+      column: 6,
+      processedBy: 'sassMatcher'
+    })
+  })
+
   it.skip('should output a sensible default', () => {
     expect(getErrorDetails(unixCwd, {
       message: 'something',