From 3b5627b04072b1d6703ef5ba782a3a0f26fd2a60 Mon Sep 17 00:00:00 2001 From: Jun Yang Date: Mon, 17 Jun 2024 22:48:57 +0800 Subject: [PATCH] feat: support catching all errors, #220 (#710) --- src/liquid-options.ts | 2 + src/parser/parser.ts | 11 ++- src/render/render.ts | 11 ++- src/util/error.ts | 15 +++- test/integration/util/error.spec.ts | 107 ++++++++++++++++++++-------- test/stub/util.ts | 6 ++ 6 files changed, 115 insertions(+), 37 deletions(-) create mode 100644 test/stub/util.ts diff --git a/src/liquid-options.ts b/src/liquid-options.ts index e7841254c5..ca5340b4dd 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 74cd10f5dd..e1b30ebbfc 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 bb64bee7fd..4949f729f6 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 139faaa485..0e169129ab 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) diff --git a/test/integration/util/error.spec.ts b/test/integration/util/error.spec.ts index 9f4c14e670..bcd50aadb3 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 0000000000..6414fb527d --- /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') +}