tschaub / mock-fs

Configurable mock for the fs module
https://npmjs.org/package/mock-fs
Other
906 stars 86 forks source link

Different behavior with yauzl #352

Open regseb opened 2 years ago

regseb commented 2 years ago

The yauzl library has a different behavior with mock-fs.

import mock from "mock-fs";
import yauzl from "yauzl";

if (process.argv.includes("mock")) {
    mock({ "baz.zip": mock.load("baz.zip") });
}

const zipfile = await new Promise((resolve, reject) => {
    yauzl.open("baz.zip", (err, zipfile) => {
        if (err) reject(err);
        resolve(zipfile);
    });
});
zipfile.on("error", (err) => console.log("error", err));
zipfile.on("entry", (entry) => console.log("entry", entry.fileName));
zipfile.on("end", () => console.log("end"));

The above code prints :

I think the problem is that mock-fs does not call IO and returns the result directly (Event Loop). Because without the promise, the behavior is the same.

yauzl.open("baz.zip", (err, zipfile) => {
    if (err) throw err;
    zipfile.on("error", (err) => console.log("error", err));
    zipfile.on("entry", (entry) => console.log("entry", entry.fileName));
    zipfile.on("end", () => console.log("end"));
});

To reproduce the problem:

regseb commented 2 years ago

@3cp

The scripts have a mock parameter to activate mock-fs. node fail.js launches the reading of the zip without anything mocked. node fail.js mock launches the reading of the zip with mock-fs. success.js works with and without mock-fs. But fail.js doesn't work with mock-fs.

node fail.js node fail.js mock node success.js node success.js mock
entry foo.txt
end
end
entry foo.txt
end
entry foo.txt
end
3cp commented 2 years ago

My bad, I missed the mock argument. I can reproduce the issue on both node v14 and v16.

3cp commented 2 years ago

I don't quite understand this Nodejs (or JavaScript) feature, the promise that handles events "error", "entry" and "end". I thought only the reject and resolve calls interact with the promise itself? Is there some official document around the wrapped promise becoming an event target?

regseb commented 2 years ago

The variable globalZipfile contains the value of the variable callbackZipfile. The await waits the promise and returns the fulfilled value of the promise (or throws an error if the promise is rejected).

const globalZipfile = await new Promise((resolve, reject) => {
    yauzl.open("baz.zip", (err, callbackZipfile) => {
        if (err) reject(err);
        resolve(callbackZipfile);
    });
});
regseb commented 2 years ago

Here is another test case using setImmediate() that fails with mock-fs.

import mock from "mock-fs";
import yauzl from "yauzl";

if (process.argv.includes("mock")) {
    mock({ "baz.zip": mock.load("baz.zip") });
}

yauzl.open("baz.zip", (err, zipfile) => {
    if (err) throw err;
    setImmediate(() => {
        zipfile.on("error", (err) => console.log("error", err));
        zipfile.on("entry", (entry) => console.log("entry", entry.fileName));
        zipfile.on("end", () => console.log("end"));
    });
});
3cp commented 2 years ago

Oh, I see. I got confused. It's the resolved value in play.

3cp commented 2 years ago

Feels like another timing issue because of our readStream patch. The zipfile already emitted those entry events before the promise exercise the .then(resolved => {...}). The lazyEntries mode of yauzl seems work as expected.