harshankur / officeParser

A Node.js library to parse text out of any office file. Currently supports docx, pptx, xlsx and odt, odp, ods..
MIT License
123 stars 17 forks source link

(Bug) Uncatchable errors caused by internalCallback #23

Closed hvm2hvm closed 9 months ago

hvm2hvm commented 9 months ago

Hi,

Using this code and the attached file, one can check that there's an uncatchable error caused by the officeParser library, for both the callback and promise variants.

const officeParser = require("officeparser");

setTimeout(()=>{console.log("didn't crash");}, 2000);

try {
    officeParser.parseOffice("empty.xlsx", (res, err) => {
        if (err) {
            console.log("officeparser: error: ", err);
        } else {
            console.log("officeparser: no error, res: ", res);
        }
    });
} catch (err) {
    console.log("caught error", err);
}

try {
    officeParser.parseOfficeAsync("empty.xlsx").then(res => {
        console.log("got res: ", res);
    }).catch(err => {
        console.log("caught promise error", err);
    });
} catch (err) {
    console.log("caught error", err);
}

empty.xlsx

(The file itself is irrelevant, any error that appears during parsing will create the same issue)

If I change the internalCallback function (line 533) so that it looks like this, both of the use cases above fail gracefully.

function internalCallback(data, err) {
    // Check if we need to preserve unzipped content files or delete them.
    if (!internalConfig.preserveTempFiles)
        // Delete decompress sublocation.
        rimraf(internalConfig.tempFilesLocation, rimrafErr => consoleError(rimrafErr, internalConfig.outputErrorToConsole));

    // Check if there is an error. Throw if there is an error.
    if (err) {
        callback(undefined, err);
    } else {
        // Call the original callback
        callback(data, undefined);
    }
}

(The existing implementation is throw-ing an error instead of calling the callback which is not catchable in any way: https://github.com/harshankur/officeParser/blob/1ad0832227a490be7bb9b2278ba1572b17dfff38/officeParser.js#L541)

harshankur commented 9 months ago

@hvm2hvm Hi, I am sorry for this error. This has already been fixed a couple of days ago. Can you please test with the latest version of OfficeParser v4.0.5? Please reopen this issue again if it has not been fixed.