Error when running the same report twice

[wiseolddj]

We are using the following configuration:

jsreport.use(
    require('jsreport-handlebars')())
    .use(
        require('jsreport-xlsx')()
    ).use(
    require('jsreport-templates')());

We then have a function that takes the template object that returns the following promise:

const renderExcelReport = (template) => jsreport.init()
    .then(() => jsreport.documentStore.collection('xlsxTemplates')
        .insert({
            contentRaw: template.template.xlsxTemplate.content,
            shortid: template.template.xlsxTemplate.shortid,
            name: template.template.xlsxTemplate.name
        }))
    .then(() => jsreport.render(template));

This works when we do a single run. The second time we run the code we get the following error:

Error when processing render request Error during rendering report: Unsupported module in tasks: path. To enable require on particular module, you need to update the configuration as {"tasks": { "allowedModules": ["path"] } } ... Alternatively you can also set "*" to allowedModules to enable everything 

If I then set allowedModules to "*" I get a different error on the second time:

TypeError: (intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(...) is not a function

I can recreate the error by running jsreport.init() after brining in JSReport.

Any ideas on where I am going wrong?

Thanks

[bjrmatos]

hi @wiseolddj , i was not able to reproduce your issue with your steps, can you give us a more complete example (using the minimal possible code and with real data)? maybe it is best if you can create a repository with the basic code and data so we can quickly run and verify the issue.

also, please give us your OS, node version, npm version, that info will give us more information about the problem and try to reproduce in an identical environment.

[wiseolddj]

Example can be found here: https://github.com/wiseolddj/js-report-test

Running on mac:

 System Version: OS X 10.11.6 (15G31)
 Kernel Version: Darwin 15.6.0

$ node --version
v6.10.2

$ npm --version
3.10.10

Thanks for looking at this.

[bjrmatos]

@wiseolddj thnks, your repository really helped me to quickly find the problems.

first problem: Error when processing render request Error during rendering report: Unsupported module in tasks: path. To enable require on particular module, you need to update the configuration as {"tasks": { "allowedModules": ["path"] } } ... Alternatively you can also set "*" to allowedModules to enable everything

this was caused because you are calling jsreport.initmultiple times, when the expected usage is that this function should only be called one time per application running. (see "Updated code with correct usage and without issues" section to know how to ensure that initialization occurs just one time)

@pofider maybe we can introduce a check in init:

Reporter.prototype.init = function () {
  if (this._initialized) {
    return Promise.resolve(this);
  }

  /* ..rest of code.. */

to always ensure that a jsreport instance can't be initialized multiple times, first call will do its job normally, second and next calls will just return the instance directly. with this check in place, other users won't fall in this problem.

second problem: TypeError: (intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(intermediate value)(...) is not a function

this problem is mostly jsreport fault, we are modifying the object that is passed to .render(obj) function in multiples places (mostly in extensions).

for example in your case you are using the same render options for multiple renders, in every render, jsreport and extensions adds/updates some properties in the options object, so in the second render your template variable has some internal properties produced by the first render, the values of those properties added are causing the second error that you have.

solution: just pass a new options object when rendering (see "Updated code with correct usage and without issues" for an example)

@pofider not sure what is the best to do here, some options:

• cloning the request object in this line before starting the render pipeline to make sure that we don't mutate the original object passed from the user... not sure if this is going to work because i'm not 100% sure that a request object (from express) can be cloned correctly, and also this will probably add some overhead and cost to renders.
• just add in jsreport docs that to safely render something, user must pass a new options object (not reusing a variable).
• try to address this problem with a refactor in jsreport v2

Updated code with correct usage and without issues

'use strict'

const fs = require('fs');
const path = require('path');
const jsreport = require('jsreport-core')();

// variable to make sure that we start jsreport only one time
let jsreportStarted = false

jsreport.use(
    require('jsreport-handlebars')())
    .use(
        require('jsreport-xlsx')()
    ).use(
    require('jsreport-templates')());

const renderExcelReport = (template) => {
    let init

    if (jsreportStarted) {
        init = Promise.resolve(jsreport)
    } else {
        console.log('initializing jsreport...')
        jsreportStarted = true
        init = jsreport.init()
    }

    return (
        init
        .then(() => jsreport.documentStore.collection('xlsxTemplates')
            .remove({name: template.template.xlsxTemplate.name}))
        .then(() => jsreport.documentStore.collection('xlsxTemplates')
            .insert({
                contentRaw: template.template.xlsxTemplate.content,
                shortid: template.template.xlsxTemplate.shortid,
                name: template.template.xlsxTemplate.name
            }))
        .then(() => jsreport.render(template))
    );
}

// function to make sure that we always use a new options object in every render
function getTemplateOptions () {
  return {
      template: {
          content: fs.readFileSync(path.resolve('./template.hbs')).toString('utf-8'),
          recipe: 'xlsx',
          engine: 'handlebars',
          helpers: 'function add(a, b){return a + b;}',
          xlsxTemplate: {
              shortid: 'test',
              name: 'testTemplate',
              content: fs.readFileSync(path.resolve('./template.xlsx')).toString('base64')
          },
      },
      data: {
         cellOne: "NEW TEXT"
      }
  };
}

renderExcelReport(getTemplateOptions())
.then(() => {
    console.log('RUNNING ONCE WAS OK')
    return renderExcelReport(getTemplateOptions())
    .then(() => console.log('RUNNING TWICE WAS OK'))
    .catch((err) => { console.log('RUNNING TWICE WAS NOT OK'); console.log(err) });
})
.catch((err) => console.log(err));

[pofider]

maybe we can introduce a check in init

Hm. We would for sure need to return a previous call promise. However I think it is the best to throw error. If you agree, we would mark it for release 2

this problem is mostly jsreport fault, we are modifying the object that is passed to .render(obj) function in multiples places (mostly in extensions).

Yes, this is ugly. We should clone it with Object.assign. However this will not work now, we should do it during the https://github.com/jsreport/jsreport/issues/262

[wiseolddj]

Hello,

Thanks for your help.

I ended up creating a function around init to ensure we only trigger it once:

let jsreportStarted = false;
const initReport = () => {
    if (jsreportStarted) {
        return Promise.resolve(jsreport);
    }
    logger.info('initializing jsreport');
    jsreportStarted = true;
    return jsreport.init();
};

It might be useful to have this within the init function as suggested above. I say this because my deployment is to AWS lambda so I don't know if init has been called or not as AWS lambda keeps environments for short amounts of time.

[bjrmatos]

Hm. We would for sure need to return a previous call promise. However I think it is the best to throw error. If you agree, we would mark it for release 2

hmm yes, seems like it is fair enough to throw an error in that case 👌

i'm going to close the issue and add the task for init in our list for v2

thnks @wiseolddj for reporting this!