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

Error handling #575

Open
binarythinktank opened this issue Jan 28, 2021 · 2 comments · May be fixed by #640
Open

Error handling #575

binarythinktank opened this issue Jan 28, 2021 · 2 comments · May be fixed by #640

Comments

@binarythinktank
Copy link

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
Copy link

blutorange commented Nov 27, 2021

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
Copy link
Owner

mde commented Nov 27, 2021

I would definitely consider a PR that implements this.

blutorange added a commit to blutorange/ejs that referenced this issue Nov 28, 2021
blutorange added a commit to blutorange/ejs that referenced this issue Nov 28, 2021
blutorange added a commit to blutorange/ejs that referenced this issue Jan 14, 2023
blutorange added a commit to blutorange/ejs that referenced this issue Jan 14, 2023
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

Successfully merging a pull request may close this issue.

3 participants