Error handling #575

binarythinktank · 2021-01-28T12:24:45Z

i have unpredictable data objects going into ejs and it seems if its missing a var it throws a "path":"" error and fails to render the template. is there a way to get ejs to ignore errors? as its a template system i would prefer it just to output nothing if a variable doesnt exist and move on with the rest of the template. not to die and cancel rendering the entire template.

it would also be useful if it could output something more descriptive than "path":"" :)

thanks!

blutorange · 2021-11-27T13:36:24Z

I am going to second this.

Our use case: I'm evaluating whether we can use EJS for an app that allows users to enter custom templates. For example, users can configure the system to send emails, and they should be able to enter a template for the email's content. Users make mistakes and we need to expect users to enter invalid templates. The entire template should not fail when a single expression in a <%= ... %> is invalid. Syntax errors can be checked at compile time, but semantic errors cannot be:

Hello <%= usr.gender === 'female' ? 'Mrs' : 'Mr' %] <% user.firstName %> <%= user.lastName %>
...content of the email...

The form of address will fail at runtime (usr instead of user), and currently, this will result in the entire template failing. But we need the app to be more resilient to these errors, the form of address may be missing, but it's still very useful to send the email with the remainder of the content. We may need to log an error and tell the user to fix the template, but a single mistake should not completely break the template.

Not sure what the best solution here is. I took a quick look at what could be done, and one thing I realized is that catching errors is possible only for expression (e.g. <%= ... %>), not for statements. Statements can be partial JS code, such as <% if (...) { %> more stuff in between <% } %>. Automatically wrapping these in a try-catch may be possible, but not really feasible, as you'd have to parse the code (and would you catch all errors, or only parts etc.?)

Perhaps you could introduce an option like errorFn. When set, EJS automatically catches errors from expressions and calls the errorFn with the error? Could be used like this:

const compiled = ejs.compile("...", {
  errorFn: e => {
    console.error("Unhandled error during template evaluation", e);
    // Insert this value into the template instead
    return "ERROR!"
  },
});

Then a template like [% data.valid %] -- [%= data.invalid.bar %] would render to ValidData -- ERROR!.

A quick hacky diff I used to test whether this idea could even work:

diff --git a/lib/ejs.js b/lib/ejs.js
index aa6322e304ab6a9a63efeaaf301c792ade243d42..c78ce19a7253707ba60ab6e35e4d6cd8cfa38463 100755
--- a/lib/ejs.js
+++ b/lib/ejs.js
@@ -533,6 +533,7 @@ function Template(text, opts) {
   options.async = opts.async;
   options.destructuredLocals = opts.destructuredLocals;
   options.legacyInclude = typeof opts.legacyInclude != 'undefined' ? !!opts.legacyInclude : true;
+  options.errorFn = opts.errorFn ?? undefined;

   if (options.strict) {
     options._with = false;
@@ -576,6 +577,7 @@ Template.prototype = {
     var appended = '';
     /** @type {EscapeCallback} */
     var escapeFn = opts.escapeFunction;
+    var errorFn = opts.errorFn;
     /** @type {FunctionConstructor} */
     var ctor;
     /** @type {string} */
@@ -615,7 +617,7 @@ Template.prototype = {
         + 'try {' + '\n'
         + this.source
         + '} catch (e) {' + '\n'
-        + '  rethrow(e, __lines, __filename, __line, escapeFn);' + '\n'
+        + '  rethrow(e, __lines, __filename, __line, escapeFn, errorFn);' + '\n'
         + '}' + '\n';
     }
     else {
@@ -624,6 +626,7 @@ Template.prototype = {

     if (opts.client) {
       src = 'escapeFn = escapeFn || ' + escapeFn.toString() + ';' + '\n' + src;
+      src = 'errorFn = errorFn || ' + errorFn?.toString() + ';' + '\n' + src;
       if (opts.compileDebug) {
         src = 'rethrow = rethrow || ' + rethrow.toString() + ';' + '\n' + src;
       }
@@ -639,7 +642,6 @@ Template.prototype = {
       src = src + '\n'
         + '//# sourceURL=' + sanitizedFilename + '\n';
     }
-
     try {
       if (opts.async) {
         // Have to use generated function for this, since in envs without support,
@@ -659,7 +661,7 @@ Template.prototype = {
       else {
         ctor = Function;
       }
-      fn = new ctor(opts.localsName + ', escapeFn, include, rethrow', src);
+      fn = new ctor(opts.localsName + ', escapeFn, errorFn, include, rethrow', src);
     }
     catch(e) {
       // istanbul ignore else
@@ -689,7 +691,7 @@ Template.prototype = {
         }
         return includeFile(path, opts)(d);
       };
-      return fn.apply(opts.context, [data || {}, escapeFn, include, rethrow]);
+      return fn.apply(opts.context, [data || {}, escapeFn, errorFn, include, rethrow]);
     };
     if (opts.filename && typeof Object.defineProperty === 'function') {
       var filename = opts.filename;
@@ -827,8 +829,8 @@ Template.prototype = {
       this.mode = Template.modes.LITERAL;
       this.source += '    ; __append("' + line.replace(o + d + d, o + d) + '")' + '\n';
       break;
-    case d + d + c:
-      this.mode = Template.modes.LITERAL;
+      case d + d + c:
+        this.mode = Template.modes.LITERAL;
       this.source += '    ; __append("' + line.replace(d + d + c, d + c) + '")' + '\n';
       break;
     case d + c:
@@ -860,11 +862,21 @@ Template.prototype = {
           break;
           // Exec, esc, and output
         case Template.modes.ESCAPED:
-          this.source += '    ; __append(escapeFn(' + stripSemi(line) + '))' + '\n';
+          if (this.opts.errorFn) {
+            this.source += '    ; try { __append(escapeFn(' + stripSemi(line) + '))}catch(e){__append(escapeFn(errorFn(e)))}' + '\n';
+          }
+          else {
+            this.source += '    ; __append(escapeFn(' + stripSemi(line) + '))' + '\n';
+          }
           break;
           // Exec and output
         case Template.modes.RAW:
-          this.source += '    ; __append(' + stripSemi(line) + ')' + '\n';
+          if (this.opts.errorFn) {
+            this.source += '    ; try{__append(' + stripSemi(line) + ')}catch(e){__append(errorFn(e))}' + '\n';
+          }
+          else {
+            this.source += '    ; __append(' + stripSemi(line) + ')' + '\n';
+          }
           break;
         case Template.modes.COMMENT:
           // Do nothing

mde · 2021-11-27T18:08:52Z

I would definitely consider a PR that implements this.

…and 'processSource'

blutorange added a commit to blutorange/ejs that referenced this issue Nov 28, 2021

Implements mde#575 Better error handling via the new options 'error' …

d47793a

…and 'processSource'

blutorange linked a pull request Nov 28, 2021 that will close this issue

Implements #575 Better error handling via the new options 'error' and 'processSource' #640

Open

blutorange added a commit to blutorange/ejs that referenced this issue Nov 28, 2021

mde#575 Add case for error option to benchmarkRender

0f337d1

blutorange added a commit to blutorange/ejs that referenced this issue Jan 14, 2023

Implements mde#575 Better error handling via the new options 'error' …

84468fe

…and 'processSource'

blutorange added a commit to blutorange/ejs that referenced this issue Jan 14, 2023

mde#575 Add case for error option to benchmarkRender

bfb18b2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Error handling #575

Error handling #575

binarythinktank commented Jan 28, 2021

blutorange commented Nov 27, 2021 •

edited

mde commented Nov 27, 2021

Error handling #575

Error handling #575

Comments

binarythinktank commented Jan 28, 2021

blutorange commented Nov 27, 2021 • edited

mde commented Nov 27, 2021

blutorange commented Nov 27, 2021 •

edited