Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to catch errors when scss parsing fails #539

Open
notlee opened this issue Aug 12, 2019 · 2 comments
Open

Unable to catch errors when scss parsing fails #539

notlee opened this issue Aug 12, 2019 · 2 comments

Comments

@notlee
Copy link
Contributor

notlee commented Aug 12, 2019

It doesn't appear to be possible to catch a scss parsing error.

E.g. given this scss file which will currently cause a parse error:

/// no semicolon as end of file
/// parsing will fail, although this appears to be valid sass
$a: 1

sassdoc will cause an error which cannot be caught in a try/catch block:

'use strict';
const sassdoc = require('sassdoc');

(async function() {
    try {
        const doclets = await sassdoc('./example-scss', {});
        console.log({doclets});
    } catch (error) {
        // this is never reached
        console.log('sassdoc failed to generate doclets: ' + error.message);
    }
})();

Screenshot 2019-08-12 at 17 07 02

I believe this may be because the error is from a stream, and the through callback isn't called around here?

@notlee notlee changed the title Unable to catch errors from scss-comment-parser Unable to catch errors when scss parsing fails Aug 12, 2019
@notlee
Copy link
Contributor Author

notlee commented Aug 12, 2019

Here's a diff which I think would solve the problem.
Happy to PR if you agree 😄

diff --git a/src/parser.js b/src/parser.js
index c00d04b..ceb71f8 100644
--- a/src/parser.js
+++ b/src/parser.js
@@ -60,8 +60,6 @@ export default class Parser {
     let data = []
 
     let transform = (file, enc, cb) => {
-      // Pass-through.
-      cb(null, file)
 
       let parseFile = ({ buf, name, path }) => {
         let fileData = this.parse(buf.toString(enc), name)
@@ -76,21 +74,27 @@ export default class Parser {
         })
       }
 
-      if (file.isBuffer()) {
-        let args = {
-          buf: file.contents,
-          name: path.basename(file.relative),
-          path: file.relative,
+      try {
+        if (file.isBuffer()) {
+          let args = {
+            buf: file.contents,
+            name: path.basename(file.relative),
+            path: file.relative,
+          }
+          parseFile(args)
         }
 
-        parseFile(args)
+        if (file.isStream()) {
+          file.pipe(concat(buf => {
+            parseFile({ buf })
+          }))
+        }
+      } catch (error) {
+        cb(error);
       }
 
-      if (file.isStream()) {
-        file.pipe(concat(buf => {
-          parseFile({ buf })
-        }))
-      }
+      // Pass-through.
+      cb(null, file)
     }
 
     let flush = cb => {

@pascalduez
Copy link
Member

Hi @notlee,

yes, that seems like reasonable addition ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants