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

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

blutorange
Copy link

@blutorange blutorange commented Nov 28, 2021

Closes #575 Better error handling via the new options error and processSource

From the readme:

Options

  • error The error handler function used for the <%= construct and
    <%- construct. When an error is thrown within these constructs, the error handler is
    called. It receives two arguments: err (type unknown), which is the error object
    that was thrown; and escapeFn (type (text: string) => string), which is the current
    function for escaping literal text. The error function may return a string, and if it
    does, that value is inserted into the template instead.
  • processSource Callback that is invoked with the generated source code of the
    template, without the header and footer added by EJS. Can be used to transform the source.
    The callback receives two arguments: source (string), which is the generated source text;
    and outputVar (type string), which is the name of the variable that contains the template
    text and can be appended to. The callback must return a value of type string, which is the
    transformed source. One use case for this callback is to wrap all individual top-level statements
    in try-catch-blocks (e.g. by using a parser such as acorn and a stringifier such as astring)
    for improved error resilience.

Error handling

In an ideal world, all templates are valid and all JavaScript code they contain
never throws an error. Unfortunately, this is not always the case in the real
world. By default, when any JavaScript code in a template throws an error, the
entire templates fails and not text is rendered. Sometimes you might want to
ignore errors and still render the rest of the template.

You can use the error option to handle errors within expressions (the <%=%>
and <%-%> tags). This is a callback that is invoked when an unhandled error
is thrown:

const ejs = require('ejs');

ejs.render('<%= valid %> <%= i.am.invalid %>', { valid: 2 }, {
  error: function(err, escapeFn) {
    console.error(err);
    return escapeFn("ERROR");
  }
});

The code above logs the error to the console and renders the text 2 ERROR.

Note that this only applies to expression, not to other control blocks such
as <%if (something.invalid) { %> ... <% } %>. To handle errors in these cases,
you e.g. can use the processSource option to wrap individual top-level
statements in try-catch blocks. For example, by using acorn and astring
for processing JavaScript source code:

const ejs = require('ejs');
const acorn = require('acorn');
const astring = require('astring');

ejs.render('<%= valid %> <%if (something.invalid) { %> foo <% } %>',
  { valid: 2 },
  {
    // Wrap all individual top-level statement in a try-catch block
    processSource: function(source, outputVar) {
      const ast = acorn.parse(source, {
        allowAwaitOutsideFunction: true,
        allowReturnOutsideFunction: true,
        ecmaVersion: 2020,
      });
      return ast.body
        .filter(node => node.type !== "EmptyStatement")
        .map(node => {
          const statement = astring.generate(node, { indent: "", lineEnd: "" });
          switch (node.type) {
            case "ReturnStatement":
            case "TryStatement":
            case "EmptyStatement":
              return statement;
            default:
              return `try{${statement}}catch(e){console.error(e);${outputVar}+='STATEMENT_ERROR'}`;
          }
        })
        .join("\n");
  },
});

The above code logs the error to the console and renders the text 2 STATEMENT_ERROR.

@shimonbrandsdorfer
Copy link

When I check out this PR and run tests with mocha, I get 3 failing tests.
Am I missing anything? Or this is not fully implemented yet?

@blutorange
Copy link
Author

I think it was done, but this PR is from some time ago, things may have changed in the mean time. I'd have to take a look at the failing tests.

@shimonbrandsdorfer
Copy link

Here is attached a screenshot of 3 failing tests.

ejs-fail

@blutorange
Copy link
Author

blutorange commented Jan 14, 2023

@shimonbrandsdorfer

I checked out this branch and ran the tests, there was only 1 test failure; and that was because it was checking an error message that has changed in node 16 -- with node 14 all tests are passing for me.

nvm use 14
git checkout 0f337d1f8aa5e56794a8a55d54be58750fe6e3f1
npm install
# had to fix some code style issues
node node_modules/.bin/eslint "**/*.js" --fix
npx jake test

Then I also rebased against the current main branch. Here the tests are also all passing.

nvm use 16
git checkout issue-575-error-handling
npm install
# npx jake test results in an error "ReferenceError: suite is not defined" 
node ./node_modules/.bin/mocha -u tdd

I'm running on a Linux (Ubuntu) OS, perhaps there are some issues with the tests on Windows? Have you tried running the tests from the main branch? nvm, the tests that are failing for you are the newly added tests

@blutorange
Copy link
Author

blutorange commented Jan 14, 2023

One of the failing tests has e instanceof ReferenceError. It seems that is also calls the error handler for you, but with a different error type. That might have to do with your environment / the node runtime? Perhaps you could change the test client mode supports error handler in client mode (file test/ejs.js) and see what type of error it gives you?

  test('supports error handler in client mode', function () {
    assert.equal(ejs.render('<%= it.does.not.exist %>', {}, {
      client: true, error: function(e){return e.toString();}
    }), 'true&amp;');
  });

For me, that prints -ReferenceError: it is not defined. But as long as the error handler is called, this PR is working as intended and it's probably the test itself that needs fixing.

image

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 this pull request may close these issues.

Error handling
2 participants