NathanaelA / fluentreports

:page_facing_up: Fluent Reports - Data Driven Reporting Engine for Node.js and Browsers :page_facing_up:
https://fluentreports.com
MIT License
375 stars 84 forks source link

PDFKit RangeError: Maximum call stack size exceeded #267

Closed 97andrewvaldez closed 1 year ago

97andrewvaldez commented 1 year ago

Background

Puzzling one here. Might be difficult to replicate but wanted to see if our solution will wreck things down the line.

We have an express reports router with multiple get requests to run and return reports, ie. /reports/account, /reports/fund, /reports/vendor.

(simplified/removed some lines to leave out company data)

const router = express.Router(); 
router.get('/account', async (req: TokenTypedRequestBody<CompanyBaseRequest>, res) => {
    const accounts = await getData();
    const company = await getCompany();
    const buffer = await generateAccountChartReport(company.company, accounts);
    res.status(200).send(successResponse(buffer.toString('base64')));
});

router.get('/fund', verifyToken, async (req: TokenTypedRequestBody<CompanyBaseRequest>, res) => {
  const funds = await getFunds();
  const company = await getCompany();
  const buffer = await generateFundReport(company.company, funds);
  res.status(200).send(successResponse(buffer.toString('base64')));
});

router.get('/vendor', verifyToken, async (req: TokenTypedRequestBody<VendorListRequest>, res) => {
  const vendors = await getVendors();
  const company = await getCompany();
  const buffer = await generateVendorListReport(company.company, vendors);
  res.status(200).send(successResponse(buffer.toString('base64')));
});
 export default router;

Then the report generation was done like so:

const Report = require('fluentReports').Report;
export async function generateVendorListReport(companyName: string, rptData: Vendor[]): Promise<void> {
  const header = function (rpt: any) {
    rpt.print(companyName);
    rpt.print('Page ' + rpt.currentPage(), {align: 'right'});
    rpt.line(rpt.minX(), 60, rpt.maxX(), 60);
    rpt.newLine();
  };

  const detail = function (rpt: any, data: Vendor) {
    rpt.band(
      [
        {data: data.vendorNumber, width: 50},
        {data: '', width: 5},
        {data: data.name, width: 238, boarder: 1},
      ],
       {border: 0, wrap: 1}
    );
    rpt.band(
      [
        {data: '', width: 55},
        {data: data.phoneNumber, width: 238, boarder: 1},
      ],
      {border: 0, wrap: 1}
    );
    rpt.newLine(1);
  };

  const rpt = new Report('buffer');
  rpt.data(rptData).margins(30).pageHeader(header).detail(detail);

  return new Promise((resolve, reject) => {
    rpt.render((err: any, resultBuffer: any) => {
      if (!err) {
        console.log('PDF Buffer ', resultBuffer);
        resolve(resultBuffer);
      } else {
        reject(err);
      }
    });
  });
}

The Issue

When all 3 of these API routes were visible we would get a maximum call stack size exceeded error when trying to run a single report from a route (ie. reports/account).

But when commenting out 2 of the 3 routes the one uncommented route would work just fine and return the report - no matter which one it was. This may be something we were doing wrong in node as well, but I wasn't sure what would have caused that. I thought maybe all 3 were trying to run when one route was being called but verified that breakpoints weren't being hit.

Source

Traced the stack overflow to fluentreports/lib/fluentReports.pdfkit.js lines 1018-1047

// ---------------------------------------------------------------------------
// We need to Hijack the PDFKit Document End and _Finalize to support deletion and auto-deletion
// ---------------------------------------------------------------------------
//noinspection JSUnresolvedVariable
pdfkit.prototype._PKIEnd = pdfkit.prototype.end;

noinspection JSUnresolvedVariable
 pdfkit.prototype.end = function () {
     if (this.isEmptyPage()) {
         this.deletePage();
     }
     this._PKIEnd();
 };

//noinspection JSUnresolvedVariable
pdfkit.prototype._PKIFinalize = pdfkit.prototype._finalize;

//noinspection JSUnresolvedVariable
 pdfkit.prototype._finalize = function (fn) {
     let i = 0;
     while (i < this._offsets.length) {
         // Eliminate any NULL Offsets from deleted pages
         if (this._offsets[i] === null) {
             this._offsets.splice(i, 1);
             continue;
         }
         i++;/
     }
     this._PKIFinalize(fn);
 };

The end() function was being called, and isEmptyPage() would return false, then PKIEnd()->end()->PKIEnd() over and over until the stack overflow occurred. I commented this out and then the finalize function did the same thing, calling in a loop finalize->PKIFinalize->finalize->PKIFinalize etc.

After commenting out both of these overwritten functions and everything worked just fine. Note that when I had the only one API Route active at a time these functions would just be called once than exit how they were supposed to.

Solution

Commenting out the overwritten functions

// ---------------------------------------------------------------------------
// We need to Hijack the PDFKit Document End and _Finalize to support deletion and auto-deletion
// ---------------------------------------------------------------------------
//noinspection JSUnresolvedVariable
pdfkit.prototype._PKIEnd = pdfkit.prototype.end;

//noinspection JSUnresolvedVariable
// pdfkit.prototype.end = function () {
//     if (this.isEmptyPage()) {
//         this.deletePage();
//     }
//     this._PKIEnd();
// };

//noinspection JSUnresolvedVariable
pdfkit.prototype._PKIFinalize = pdfkit.prototype._finalize;

//noinspection JSUnresolvedVariable
// pdfkit.prototype._finalize = function (fn) {
//     let i = 0;
//     while (i < this._offsets.length) {
//         // Eliminate any NULL Offsets from deleted pages
//         if (this._offsets[i] === null) {
//             this._offsets.splice(i, 1);
//             continue;
//         }
//         i++;/
//     }
//     this._PKIFinalize(fn);
// };

Now this worked for us locally but I'm slightly concerned it will break things in the future - will this just trim empty pages from the end of reports?

This could also be an issue with the way we setup express, node, and routes since commenting out some routes fixed the others, but I thought I'd post it here in case anyone else had this issue.

Loving the library though, everything else has been very quick and easy.

Cheers!

NathanaelA commented 1 year ago

Your issue is very weird. There are two possibilities I can think of (the second being more likely)...

And yes, you are correct this code just eliminates basically empty stuff, however I think w/o the _finalize code you might randomly get output glitches occasionally as null offsets would probably be translated to 0,0 offsets...

  1. Can you tell me which version of PDFKit that you are using, maybe an upstream change in it broke something...

  2. The main thing I can think of is that somehow the FluentReports.pdfkit.js overrides is being initialized multiple times and replacing _end & _finalize a second time in the PDFKit, which would for sure create a loop like you are seeing. Could you be using two different copies of FluentReports or perhaps FR is npm installed in two (or more) directories, or perhaps their is some weird issue with bundling or node that is causing the fluentreports.pdfkit.js to be loaded a second time. In any case, changing the code in all copies of fluentreports/lib/fluentReports.pdfkit.js to the following should eliminate that possibility...

if (!pdfkit.prototype._PKIEnd) {
 pdfkit.prototype._PKIEnd = pdfkit.prototype.end;

  pdfkit.prototype.end = function () {
     if (this.isEmptyPage()) {
         this.deletePage();
     }
     this._PKIEnd();
 };
}

if (!pdfkit.prototype._PKIFinalize) {
 pdfkit.prototype._PKIFinalize = pdfkit.prototype._finalize;
 pdfkit.prototype._finalize = function (fn) {
     let i = 0;
     while (i < this._offsets.length) {
         // Eliminate any NULL Offsets from deleted pages
         if (this._offsets[i] === null) {
             this._offsets.splice(i, 1);
             continue;
         }
         i++;/
     }
     this._PKIFinalize(fn);
 };
}
97andrewvaldez commented 1 year ago

Thanks for the quick response. That code snippet worked!

It also prompted me to test a bit more to see if I could optimize how those routes are loading.

I found that using const Report = require('fluentReports').Report; at the top of my report generation files caused the overrides to be initialized in each file (then would later cause the stack overflow).

then replacing const Report = require('fluentReports').Report; with import { Report } from 'fluentReports'; fixed the issue without your code changes.

FluentReports was up to date and using pdfkit .13 - for reference I also tried previous versions of fluentreports that used pdfkit .12 and .11 but had the same issue. Also pdfkit isn't installed anywhere else in our lock file so no conflicts there.

Either way that helped solve things, and eased my mind a bit 😄 thanks again!

NathanaelA commented 1 year ago

Awesome, thanks for the confirmation. I'll upload a new version of the code as that monkey patching should actually have been safeguarded to make sure people can't accidentally do what you did. Thanks for the report and it is now fixed in v1.4.1