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

Access the list of render errors #220

Closed
bbugh opened this issue Apr 21, 2020 · 8 comments
Closed

Access the list of render errors #220

bbugh opened this issue Apr 21, 2020 · 8 comments

Comments

@bbugh
Copy link

bbugh commented Apr 21, 2020

Hi! 👋 This seems like a great library.

We're using liquid-ruby for our customers to be able to customize some of their templates. We want to use liquidjs to do client-side previews and error parsing, so we can give the customers fast feedback while avoiding a round-trip to our server's precious resources.

In Ruby liquid, when rendering a template, an array of the errors is available afterwards, which can be used to display to the client. I've looked through the docs/repo, and liquidjs seems to behave very differently from liquid-ruby in this way.

Ruby example:

options = options = { strict_variables: true, strict_filters: true }
template = Liquid::Template.parse(markup)
result = template.render(context, options)

# Lots of errors to look at here
template.errors.each do |error|
  puts error
end

With the same markup and options as the Ruby version, liquidjs throws one error and quits.

const engine = new Liquid()
const template = engine.parse(markup)

// This throws a single error
const result = engine.renderSync(template, {}, { strictFilters: true, strictVariables: true })

I'm surprised this doesn't exist yet, but I'm guessing liquidjs is used more for server templates?

Is this possible to do with liquidjs or are we out of luck? Thanks!

@harttle
Copy link
Owner

harttle commented Apr 22, 2020

Both of the parse and render process are sequential, which means previous errors imply the following templates can be parsed/rendered incorrectly. In other words, only the first error can be guaranteed to be correct so it's thrown directly in the current implementation.

It requires an error recovery mechanism to allow detecting all errors without termination. There's no such plan currently but I'll keep this issue open to see how many people are looking for it.

@bbugh bbugh changed the title Access the list of parse and render errors Access the list of render errors Apr 22, 2020
@bbugh
Copy link
Author

bbugh commented Apr 22, 2020

Thank you for the reply! If I understand what you're saying correctly, that's also how I understand liquid-ruby to work: the parse errors (like malformed tags) are fatal and you only get , while render errors are not fatal, and are collected in an array you can access after. I incorrectly added "parse errors" to the title, which I fixed now, sorry for the confusion.

The list of errors we're looking for are the render-specific ones, where an object or function are missing for example. Presently liquidjs handles this by outputting nothing (just like liquid-ruby) but it looks like there's no way to access these "output nothing" errors, and that's what we're looking for.

Given this:

{{ bad_object }}
{{ another_bad_object }}

liquid-ruby would have an error array, like this psuedo-example:

[
  "Liquid error (line 1): undefined variable 'bad_object'", 
  "Liquid error (line 2): undefined variable 'another_bad_object'"
]

Does that clear it up? Is this presently possible and/or plausible?

@harttle
Copy link
Owner

harttle commented Apr 23, 2020

Missing variables and filters will render to empty by default, you'll need to set strictFilters and strictVariables options to catch these errors.

Again, it's not a list and will throw upon the first error. LiquidJS does not distinguish fatal or non-fatal errors for now.

@bbugh
Copy link
Author

bbugh commented Apr 23, 2020

Great, thank you for confirming my assumptions. If we decide to move forward with the client-side approach, I will see about a PR. It looks like handling errors happens in renderTemplates. It seems like that could return an object instead of a string, like { result: string, errors: LiquidError[] }. So as not to break the end-user API, the render function could still return a string, but keep the errors in engine. Does that seem plausible or am I way off?

@harttle
Copy link
Owner

harttle commented Apr 24, 2020

An additional method or a catchAllErrors defaults false or failFast defaults true argument will be better, or this feature will be a breaking change.

@georgemanius
Copy link

georgemanius commented Jun 11, 2024

Hello, @harttle ! We've also encountered the need to have all the missing parameters within the template returned and logged. Do you still have a plan for implementing this by any chance? Or maybe there's an existing API to retrieve all the variables within the template?

@harttle
Copy link
Owner

harttle commented Jun 11, 2024

Yes, the list of errors is planned. Thank you for calling out this again so we have better understanding of what’s most needed.

For list of variables please go #707

harttle added a commit that referenced this issue Jun 17, 2024
harttle added a commit that referenced this issue Jun 17, 2024
harttle added a commit that referenced this issue Jun 17, 2024
harttle added a commit that referenced this issue Jun 17, 2024
github-actions bot pushed a commit that referenced this issue Jun 17, 2024
# [10.14.0](v10.13.1...v10.14.0) (2024-06-17)

### Bug Fixes

* use drop `valueOf` when evaluated as condition ([#705](#705)) ([a7da93f](a7da93f))

### Features

* support catching all errors, [#220](#220) ([#710](#710)) ([3b5627b](3b5627b))
@harttle
Copy link
Owner

harttle commented Jun 17, 2024

The list of errors is released in v10.14.0, please check the caveats as described in #710

Added some use cases in

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'
}]
})
})
})

Thank you guys for driving this feature for the last 4 years, closing. Please feel free to create specific issues should you have trouble using this feature.

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

No branches or pull requests

3 participants