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

Improve error messaging #85

Open
uncenter opened this issue Nov 19, 2024 · 3 comments
Open

Improve error messaging #85

uncenter opened this issue Nov 19, 2024 · 3 comments

Comments

@uncenter
Copy link
Contributor

From my experience so far with Vento, the experience has generally been great but errors seem to be a pain point. Because of how Vento translates to JavaScript and then feeds that to meriyah, I'm often seeing errors along the lines of [2:34-2:40]: Expected ')' (via SyntaxError) coming from meriyah that is caused by some other issue (i.e. noelforte/eleventy-plugin-vento#69). Would love to see clearer messaging here.

@oscarotero
Copy link
Collaborator

oscarotero commented Nov 20, 2024

Yep, generating clear errors is probably the hardest thing because the template code is not entirely checked by Vento. The javascript parts are not parsed but executed by the runtime and it's not easy to map the position of the error produced in the final javascript code with the original template code. To do this right, we should generate a source map, but this makes the template engine much more complex.

The way this is implementing is updating the __pos variable before every tag. For example, the following template:

Text 1
{{ 1 }}
Text 2
{{ 2 }}

Generates the following javascript code (modified for clarity):

__exports.content += "Text 1\n";
__pos = 7;
__exports.content += 1;
__exports.content += "\nText 2\n";
__pos = 22;
__exports.content += 2;

The __pos variable indicates the position of the next tag in the original template code, so Vento knows the point in the template where the error has originated by looking the value of the __pos variable, that was updated just before executing the tag. This number is used to show the code of the tag when the error is thrown.

But the code transformed by meriyah doesn't use the __pos variable, the position of the error is in the final JavaScript code. To mitigate this, I just implemented this logic:

  • Using lastIndexOf, we can get the position of the last update of the __pos variable before the error occurs.
  • This allows to recover the value of this variable and generate a better error showing the original code causing the error.

Let me know if this works before releasing a new version.

@noelforte
Copy link
Contributor

noelforte commented Nov 21, 2024

Just built and tried integrating the latest HEAD. I'm curious to hear what @uncenter thinks, in their case the error was getting thrown because of a missing custom tag not loaded by the Eleventy/Vento tag factory, rendering the transformed code invalid, hence the thrown error on a missing ).

The new error reporting is definitely better but still is a bit misleading. In this case, the error correctly identifies the part of the template causing the problem, but it's not clear why/where adding a ) will fix the problem:

~/D/t/vento-11ty-repro => pnpm build

> @ build /***/tests/vento-11ty-repro
> eleventy

[11ty] Problem writing Eleventy templates:
[11ty] 1. Having trouble rendering vto template ./src/index.vto (via TemplateContentRenderError)
[11ty] 2. Having trouble compiling template ./src/index.vto (via TemplateContentCompileError)
[11ty] 3. Error in the template src/index.vto:1:1
[11ty] 
[11ty] {{ myShortcode 'arg1', 'arg2' }}
[11ty] 
[11ty] > [2:34-2:40]: Expected ')'
[11ty] 
[11ty] Original error stack trace: Error: Error in the template src/index.vto:1:1
[11ty] 
[11ty] {{ myShortcode 'arg1', 'arg2' }}
[11ty] 
[11ty] > [2:34-2:40]: Expected ')'
[11ty] 
[11ty]     at Environment.createError (file:///***/vento/_npm/esm/src/environment.js:180:16)
[11ty]     at Environment.compile (file:///***/vento/_npm/esm/src/environment.js:55:32)
[11ty]     at Object.getTemplateFunction (file:///***/eleventy-plugin-vento/dist/index.js:119:22)
[11ty]     at Object.compile (file:///***/eleventy-plugin-vento/dist/index.js:226:31)
[11ty]     at CustomEngine.compile (file:///***/vento-11ty-repro/node_modules/.pnpm/@11ty+eleventy@3.0.0/node_modules/@11ty/eleventy/src/Engines/Custom.js:239:5)
[11ty] Wrote 0 files in 0.09 seconds (v3.0.0)
 ELIFECYCLE  Command failed with exit code 1.

I'm trying to think of the best way to handle the errors to present useful information to the end users. Could I capture some part of what merriyah is attempting to parse or decorate the error object in some way?

For what its worth: I tried replicating Uncenter's issue in Liquid/Nunjucks to compare what they do. Both languages throw a Unknown tag 'myShortcode' error if they encounter a token they don't recognize. This doesn't account for all cases but in the event of a missing tag/filter/configuration setting during compilation is that something Vento could catch?

@oscarotero
Copy link
Collaborator

In Vento there's no difference between tags and printing variables. If {{ foo }} fails, you don't know if foo is a variable that you want to print, or a new tag that should be registered. Liquid/Nunjucks have different syntax for tags {% %} and printing variables {{ }} so they know what each thing is, but this is something I wanted to avoid in Vento.

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

No branches or pull requests

3 participants