Repository: harttle/liquidjs
Pull request #710 (2024-06-17 14:39:32)

feat: support catching all errors, #220
Catching multiple errors before quit when `catchAllErrors` option is set. However there's some caveats.

1. It only catches all `LiquidError`, other errors like code bug are not catched in this way. So you'll need to check `err.name === 'LiquidErrors'` before using.
2. Top level tokennization errors are not catched, as there's no such concept of `Token` during this step. e.g. partial tag `{% foo`
3. Render errors should be catched perfectly. e.g. undefined variables, nonexist filters/tags, as well as errors thrown in filters/tags.


Patch:
diff --git a/src/liquid-options.ts b/src/liquid-options.ts
index e7841254c..ca5340b4d 100644
--- a/src/liquid-options.ts
+++ b/src/liquid-options.ts
@@ -32,6 +32,8 @@ export interface LiquidOptions {
   strictFilters?: boolean;
   /** Whether or not to assert variable existence.  If set to `false`, undefined variables will be rendered as empty string.  Otherwise, undefined variables will cause an exception. Defaults to `false`. */
   strictVariables?: boolean;
+  /** Catch all errors instead of exit upon one. Please note that render errors won't be reached when parse fails. */
+  catchAllErrors?: boolean;
   /** Hide scope variables from prototypes, useful when you're passing a not sanitized object into LiquidJS or need to hide prototypes from templates. */
   ownPropertyOnly?: boolean;
   /** Modifies the behavior of `strictVariables`. If set, a single undefined variable will *not* cause an exception in the context of the `if`/`elsif`/`unless` tag and the `default` filter. Instead, it will evaluate to `false` and `null`, respectively. Irrelevant if `strictVariables` is not set. Defaults to `false`. **/
diff --git a/src/parser/parser.ts b/src/parser/parser.ts
index 74cd10f5d..e1b30ebbf 100644
--- a/src/parser/parser.ts
+++ b/src/parser/parser.ts
@@ -5,7 +5,7 @@ import { TopLevelToken, OutputToken } from '../tokens'
 import { Template, Output, HTML } from '../template'
 import { LiquidCache } from '../cache'
 import { FS, Loader, LookupType } from '../fs'
-import { LiquidError } from '../util/error'
+import { LiquidError, LiquidErrors } from '../util/error'
 import type { Liquid } from '../liquid'
 
 export class Parser {
@@ -31,9 +31,16 @@ export class Parser {
   public parseTokens (tokens: TopLevelToken[]) {
     let token
     const templates: Template[] = []
+    const errors: LiquidError[] = []
     while ((token = tokens.shift())) {
-      templates.push(this.parseToken(token, tokens))
+      try {
+        templates.push(this.parseToken(token, tokens))
+      } catch (err) {
+        if (this.liquid.options.catchAllErrors) errors.push(err as LiquidError)
+        else throw err
+      }
     }
+    if (errors.length) throw new LiquidErrors(errors)
     return templates
   }
   public parseToken (token: TopLevelToken, remainTokens: TopLevelToken[]) {
diff --git a/src/render/render.ts b/src/render/render.ts
index bb64bee7f..4949f729f 100644
--- a/src/render/render.ts
+++ b/src/render/render.ts
@@ -1,4 +1,4 @@
-import { toPromise, RenderError } from '../util'
+import { toPromise, RenderError, LiquidErrors, LiquidError } from '../util'
 import { Context } from '../context'
 import { Template } from '../template'
 import { Emitter, KeepingTypeEmitter, StreamedEmitter, SimpleEmitter } from '../emitters'
@@ -14,6 +14,7 @@ export class Render {
     if (!emitter) {
       emitter = ctx.opts.keepOutputType ? new KeepingTypeEmitter() : new SimpleEmitter()
     }
+    const errors = []
     for (const tpl of templates) {
       try {
         // if tpl.render supports emitter, it'll return empty `html`
@@ -22,10 +23,14 @@ export class Render {
         html && emitter.write(html)
         if (emitter['break'] || emitter['continue']) break
       } catch (e) {
-        const err = RenderError.is(e) ? e : new RenderError(e as Error, tpl)
-        throw err
+        const err = LiquidError.is(e) ? e : new RenderError(e as Error, tpl)
+        if (ctx.opts.catchAllErrors) errors.push(err)
+        else throw err
       }
     }
+    if (errors.length) {
+      throw new LiquidErrors(errors)
+    }
     return emitter.buffer
   }
 }
diff --git a/src/util/error.ts b/src/util/error.ts
index 139faaa48..0e169129a 100644
--- a/src/util/error.ts
+++ b/src/util/error.ts
@@ -8,7 +8,7 @@ import { Template } from '../template/template'
 const TRAIT = '__liquidClass__'
 
 export abstract class LiquidError extends Error {
-  private token!: Token
+  public token!: Token
   public context = ''
   private originalError?: Error
   public constructor (err: Error | string, token: Token) {
@@ -62,6 +62,19 @@ export class RenderError extends LiquidError {
   }
 }
 
+export class LiquidErrors extends LiquidError {
+  public constructor (public errors: RenderError[]) {
+    super(errors[0], errors[0].token)
+    this.name = 'LiquidErrors'
+    const s = errors.length > 1 ? 's' : ''
+    this.message = `${errors.length} error${s} found`
+    super.update()
+  }
+  public static is (obj: any): obj is LiquidErrors {
+    return obj.name === 'LiquidErrors'
+  }
+}
+
 export class UndefinedVariableError extends LiquidError {
   public constructor (err: Error, token: Token) {
     super(err, token)


Test patch:
diff --git a/test/integration/util/error.spec.ts b/test/integration/util/error.spec.ts
index 9f4c14e67..bcd50aadb 100644
--- a/test/integration/util/error.spec.ts
+++ b/test/integration/util/error.spec.ts
@@ -1,18 +1,28 @@
 import { RenderError } from '../../../src/util/error'
 import { Liquid } from '../../../src/liquid'
-import * as path from 'path'
+import { resolve } from 'path'
 import { mock, restore } from '../../stub/mockfs'
+import { throwIntendedError, rejectIntendedError } from '../../stub/util'
 
-let engine = new Liquid()
 const strictEngine = new Liquid({
   strictVariables: true,
   strictFilters: true
 })
+const strictCatchingEngine = new Liquid({
+  catchAllErrors: true,
+  strictVariables: true,
+  strictFilters: true
+})
+strictEngine.registerTag('throwingTag', { render: throwIntendedError })
+strictEngine.registerFilter('throwingFilter', throwIntendedError)
+strictCatchingEngine.registerTag('throwingTag', { render: throwIntendedError })
+strictCatchingEngine.registerFilter('throwingFilter', throwIntendedError)
 
 describe('error', function () {
   afterEach(restore)
 
   describe('TokenizationError', function () {
+    const engine = new Liquid()
     it('should throw TokenizationError when tag illegal', async function () {
       await expect(engine.parseAndRender('{% . a %}', {})).rejects.toMatchObject({
         name: 'TokenizationError',
@@ -68,43 +78,34 @@ describe('error', function () {
   })
 
   describe('RenderError', function () {
+    let engine: Liquid
     beforeEach(function () {
       engine = new Liquid({
         root: '/'
       })
-      engine.registerTag('throwingTag', {
-        render: function () {
-          throw new Error('intended render error')
-        }
-      })
-      engine.registerTag('rejectingTag', {
-        render: async function () {
-          throw new Error('intended render reject')
-        }
-      })
-      engine.registerFilter('throwingFilter', () => {
-        throw new Error('thrown by filter')
-      })
+      engine.registerTag('throwingTag', { render: throwIntendedError })
+      engine.registerTag('rejectingTag', { render: rejectIntendedError })
+      engine.registerFilter('throwingFilter', throwIntendedError)
     })
     it('should throw RenderError when tag throws', async function () {
       const src = '{%throwingTag%}'
       await expect(engine.parseAndRender(src)).rejects.toMatchObject({
         name: 'RenderError',
-        message: expect.stringContaining('intended render error')
+        message: expect.stringContaining('intended error')
       })
     })
     it('should throw RenderError when tag rejects', async function () {
       const src = '{%rejectingTag%}'
       await expect(engine.parseAndRender(src)).rejects.toMatchObject({
         name: 'RenderError',
-        message: expect.stringContaining('intended render reject')
+        message: expect.stringContaining('intended reject')
       })
     })
     it('should throw RenderError when filter throws', async function () {
       const src = '{{1|throwingFilter}}'
       await expect(engine.parseAndRender(src)).rejects.toMatchObject({
         name: 'RenderError',
-        message: expect.stringContaining('thrown by filter')
+        message: expect.stringContaining('intended error')
       })
     })
     it('should not throw when variable undefined by default', async function () {
@@ -113,8 +114,8 @@ describe('error', function () {
     })
     it('should throw RenderError when variable not defined', async function () {
       await expect(strictEngine.parseAndRender('{{a}}')).rejects.toMatchObject({
-        name: 'RenderError',
-        message: expect.stringContaining('undefined variable: a')
+        name: 'UndefinedVariableError',
+        message: 'undefined variable: a, line:1, col:3'
       })
     })
     it('should contain template context in err.stack', async function () {
@@ -131,7 +132,7 @@ describe('error', function () {
       ]
       await expect(engine.parseAndRender(html.join('\n'))).rejects.toMatchObject({
         name: 'RenderError',
-        message: 'intended render error, line:4, col:2',
+        message: 'intended error, line:4, col:2',
         stack: expect.stringContaining(message.join('\n'))
       })
     })
@@ -160,7 +161,7 @@ describe('error', function () {
       ]
       await expect(engine.parseAndRender(html)).rejects.toMatchObject({
         name: 'RenderError',
-        message: `intended render error, file:${path.resolve('/throwing-tag.html')}, line:4, col:2`,
+        message: `intended error, file:${resolve('/throwing-tag.html')}, line:4, col:2`,
         stack: expect.stringContaining(message.join('\n'))
       })
     })
@@ -182,25 +183,69 @@ describe('error', function () {
       ]
       await expect(engine.parseAndRender(html)).rejects.toMatchObject({
         name: 'RenderError',
-        message: `intended render error, file:${path.resolve('/throwing-tag.html')}, line:4, col:2`,
+        message: `intended error, file:${resolve('/throwing-tag.html')}, line:4, col:2`,
         stack: expect.stringContaining(message.join('\n'))
       })
     })
     it('should contain stack in err.stack', async function () {
       await expect(engine.parseAndRender('{%rejectingTag%}')).rejects.toMatchObject({
-        message: expect.stringContaining('intended render reject'),
+        message: expect.stringContaining('intended reject'),
         stack: expect.stringMatching(/at .*:\d+:\d+/)
       })
     })
   })
 
+  describe('catchAllErrors', function () {
+    it('should catch render errors', async function () {
+      const template = '{{foo}}\n{{"hello" | throwingFilter}}\n{% throwingTag %}'
+      return expect(strictCatchingEngine.parseAndRender(template)).rejects.toMatchObject({
+        name: 'LiquidErrors',
+        message: '3 errors found, line:1, col:3',
+        errors: [{
+          name: 'UndefinedVariableError',
+          message: 'undefined variable: foo, line:1, col:3'
+        }, {
+          name: 'RenderError',
+          message: 'intended error, line:2, col:1'
+        }, {
+          name: 'RenderError',
+          message: 'intended error, line:3, col:1'
+        }]
+      })
+    })
+    it('should catch some parse errors', async function () {
+      const template = '{{"foo" | filter foo }}'
+      return expect(strictCatchingEngine.parseAndRender(template)).rejects.toMatchObject({
+        name: 'LiquidErrors',
+        message: '1 error found, line:1, col:18',
+        errors: [{
+          name: 'TokenizationError',
+          message: 'expected ":" after filter name, line:1, col:18'
+        }]
+      })
+    })
+    it('should catch parse errors from filter/tag', async function () {
+      const template = '{{"foo" | nonExistFilter }} {% nonExistTag %}'
+      return expect(strictCatchingEngine.parseAndRender(template)).rejects.toMatchObject({
+        name: 'LiquidErrors',
+        message: '2 errors found, line:1, col:1',
+        errors: [{
+          name: 'ParseError',
+          message: 'undefined filter: nonExistFilter, line:1, col:1'
+        }, {
+          name: 'ParseError',
+          message: 'tag "nonExistTag" not found, line:1, col:29'
+        }]
+      })
+    })
+  })
+
   describe('ParseError', function () {
+    let engine: Liquid
     beforeEach(function () {
       engine = new Liquid()
       engine.registerTag('throwsOnParse', {
-        parse: function () {
-          throw new Error('intended parse error')
-        },
+        parse: throwIntendedError,
         render: () => ''
       })
     })
@@ -225,7 +270,7 @@ describe('error', function () {
     it('should throw ParseError when tag parse throws', async function () {
       await expect(engine.parseAndRender('{%throwsOnParse%}')).rejects.toMatchObject({
         name: 'ParseError',
-        message: expect.stringContaining('intended parse error')
+        message: expect.stringContaining('intended error')
       })
     })
     it('should throw ParseError when tag not found', async function () {
@@ -294,14 +339,14 @@ describe('error', function () {
       })
       engine.registerTag('throwingTag', {
         render: function () {
-          throw new Error('intended render error')
+          throw new Error('intended error')
         }
       })
     })
     it('should throw RenderError when tag throws', function () {
       const src = '{%throwingTag%}'
       expect(() => engine.parseAndRenderSync(src)).toThrow(RenderError)
-      expect(() => engine.parseAndRenderSync(src)).toThrow(/intended render error/)
+      expect(() => engine.parseAndRenderSync(src)).toThrow(/intended error/)
     })
     it('should contain original error info for {% include %}', function () {
       mock({
@@ -323,7 +368,7 @@ describe('error', function () {
         throw new Error('expected throw')
       } catch (err) {
         expect(err).toHaveProperty('name', 'RenderError')
-        expect(err).toHaveProperty('message', `intended render error, file:${path.resolve('/throwing-tag.html')}, line:4, col:2`)
+        expect(err).toHaveProperty('message', `intended error, file:${resolve('/throwing-tag.html')}, line:4, col:2`)
         expect(err).toHaveProperty('stack', expect.stringContaining(message.join('\n')))
       }
     })
diff --git a/test/stub/util.ts b/test/stub/util.ts
new file mode 100644
index 000000000..6414fb527
--- /dev/null
+++ b/test/stub/util.ts
@@ -0,0 +1,6 @@
+export function throwIntendedError () {
+  throw new Error('intended error')
+}
+export async function rejectIntendedError () {
+  throw new Error('intended reject')
+}