marcel-hofer / markdown-document

Converts markdown files to PDF documents using HTML templates.
https://marcel-hofer.github.io/markdown-document/
MIT License
3 stars 2 forks source link

renderPdfAsync hardcoded timeout fails on large documents #28

Closed christthomc closed 5 years ago

christthomc commented 5 years ago

Facing the issue that large documents take a while to render. The class "output/services/pdf-service.js" hosts a method called "renderPdfAsync". This method uses "childProcess.execFile" with a hardcoded timeout of 15 seconds.
The 15 second timeout leads to render failure on larger documents.

The creation of a 6MB pdf file could take up to 25 seconds.

The document would be created but is unreadable by any pdf viewer.

Affected Method:

    renderPdfAsync(layoutPath, outputPath, options) {
        const args = pdf_options_parser_1.wkhtmltopdfArguments(layoutPath, options);
        args.push(outputPath);
        const defer = q.defer();
        childProcess.execFile(options.wkhtmltopdfPath, args, { timeout: 15000 }, function (error, stdout, stderr) {
            const data = {
                error: error,
                stdout: stdout,
                stderr: stderr
            };
            if (error) {
                defer.reject(data);
            }
            else {
                defer.resolve(data);
            }
        });
        return defer.promise;
    }
}

Because the "options" parameter is already there, I'd suggest to introduce a new option parameter that could handle the timeout.

Example: pdfrendertimeout

Options.json:

    "marginTop": 25,
    "marginLeft": 20,
    "marginBottom": 12,
    "marginRight": 20,
    "orientation": "Portrait",
    "pageSize": "A4",
    "header": {
        "html": "header.html",
        "spacing": 10
    },
    "footer": {
        "html": "footer.html"
    },
    "parts": {
        "cover": {
            "type": "cover",
            "html": "cover.html"
        },
        "info": {
            "type": "content",
            "html": "info.html"
        },
        "toc": {
            "type": "toc",
            "xslStyleSheet": "toc.xslt"
        },
        "content": {
            "type": "content",
            "html": "content.html",
            "data": {
              "captions": ["table", "figure", "code", "formula"]
          }
        }
    },
    "pdfrendertimeout": 17000
}

Need to add the option on "output/services/options-service.js" as well.

marcel-hofer commented 5 years ago

Thanks for submitting the issue and the solution!

I changed the property name to match my (not documented) styleguide (lowerCamelCase) and merged the changes. The option wkhtmltopdfPath does not match the styleguide because that's the name of the used library.

I published a new package version with the changes on npm (v0.2.2).