ejs icon indicating copy to clipboard operation
ejs copied to clipboard

Error handling

Open binarythinktank opened this issue 4 years ago • 2 comments

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!

binarythinktank avatar Jan 28 '21 12:01 binarythinktank

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

blutorange avatar Nov 27 '21 13:11 blutorange

I would definitely consider a PR that implements this.

mde avatar Nov 27 '21 18:11 mde