marko-js / marko

A declarative, HTML-based language that makes building web apps fun
https://markojs.com/
MIT License
13.28k stars 641 forks source link

Sync render() doesn't throw correct error when include fails #1136

Closed JeroenVdb closed 3 years ago

JeroenVdb commented 5 years ago

Marko Version: 4.13.5

Details

I always try-catch when I renderToString() my templates because if I serve the server-side rendered template from my application it must be correct and complete. If a error occurs in my template.marko I get a nice render error and can catch it. But when I have a template.marko that includes a template where an error occurs then the renderToString() functions returns the HTML without the include. This way the template is not complete (the include is missing).

Expected Behavior

I would expect an error to be returned when my template + one of the includes produces an error.

Actual Behavior

Errors inside included templates are only thrown after the HTML of the main template is returned.

I would suggest to throw an error if any part of the template fails.

Additional Info ### Your Environment - Environment name and version (e.g. Chrome 39, node.js 5.4): Node 8.12 - Operating System and version (desktop or mobile): MacOS High Sierra 10.13.6 (17G65) - Link to your project: ### Steps to Reproduce This is the code: https://github.com/JeroenVdb/marko-include-bug Run `node index.js` ### Stack Trace ``` $ node index.js I have valid HTML

Jeroen

Some text BEFORE the include

Some text AFTER the include

/Users/Projects/tmpmarko/node_modules/events-light/src/index.js:83 throw error; // Unhandled 'error' event ^ Error: Render error. Exception: TypeError: Cannot read property 'title' of undefined at render (/Users/Projects/tmpmarko/included-file.marko.js:16:31) at renderer (/Users/Projects/tmpmarko/node_modules/marko/src/components/renderer.js:217:9) at safeRender (/Users/Projects/tmpmarko/node_modules/marko/src/runtime/renderable.js:6:9) at Template.render (/Users/Projects/tmpmarko/node_modules/marko/src/runtime/renderable.js:136:20) at doInclude (/Users/Projects/tmpmarko/node_modules/marko/src/taglibs/core/include-tag.js:17:27) at includeTag (/Users/Projects/tmpmarko/node_modules/marko/src/taglibs/core/include-tag.js:56:5) at wrappedRenderer (/Users/Projects/tmpmarko/node_modules/marko/src/runtime/helpers.js:217:13) at render (/Users/Projects/tmpmarko/template.marko.js:23:3) at renderer (/Users/Projects/tmpmarko/node_modules/marko/src/components/renderer.js:217:9) at Template.renderToString (/Users/Projects/tmpmarko/node_modules/marko/src/runtime/renderable.js:51:17) at AsyncStream.error (/Users/Projects/tmpmarko/node_modules/marko/src/runtime/html/AsyncStream.js:437:13) at Timeout._onTimeout (/Users/Projects/tmpmarko/node_modules/marko/src/runtime/renderable.js:17:22) at ontimeout (timers.js:460:11) at tryOnTimeout (timers.js:298:5) at Timer.listOnTimeout (timers.js:261:5) ```
DylanPiercey commented 3 years ago

Marko does propagate thrown errors, maybe this was a bug previously.

One thing though is that if you have <await> errors, you need to handle those specially with <@catch> (https://markojs.com/docs/core-tags/#await).