Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix non closed zip file on windows: use yaulz autoClose=true option #5

Merged
merged 1 commit into from Jun 24, 2020

Conversation

khelkun
Copy link
Contributor

@khelkun khelkun commented Jun 24, 2020

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 69c7c6d on khelkun:master into a4fa4ad on coderaiser:master.

@coderaiser
Copy link
Owner

Thank you @khelkun :), would be great to have tests for this :)

@khelkun
Copy link
Contributor Author

khelkun commented Jun 24, 2020

Thank you @khelkun :), would be great to have tests for this :)

Not easily testable:

  1. It's Windows filesystem specific and I assume your travis CI run on a Linux
  2. the npm test of the project fails on Windows because of this issue:
npm test

> onezip@4.2.0 test G:\VIZUA\github\node-onezip
> madrun test

> tape test/**/*.js
(node:25964) ExperimentalWarning: The ESM module loader is experimental.
(node:25964) UnhandledPromiseRejectionWarning: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only file and data URLs are supported by the default ESM loader
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:727:11)
    at Loader.resolve (internal/modules/esm/loader.js:97:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:243:28)
    at Loader.import (internal/modules/esm/loader.js:178:28)
    at importModuleDynamically (internal/modules/cjs/loader.js:1081:27)
    at exports.importModuleDynamicallyCallback (internal/process/esm_loader.js:37:14)
    at imp (G:\VIZUA\github\node-onezip\node_modules\supertape\lib\super-import.js:20:5)
    at module.exports (G:\VIZUA\github\node-onezip\node_modules\try-to-catch\lib\try-to-catch.js:7:29)
    at module.exports (G:\VIZUA\github\node-onezip\node_modules\supertape\lib\super-import.js:6:31)
    at G:\VIZUA\github\node-onezip\node_modules\supertape\bin\supertape.js:62:15
(node:25964) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on u
nhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 4)
(node:25964) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:25964) UnhandledPromiseRejectionWarning: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only file and data URLs are supported by the default ESM loader
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:727:11)
    at Loader.resolve (internal/modules/esm/loader.js:97:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:243:28)
    at Loader.import (internal/modules/esm/loader.js:178:28)
    at importModuleDynamically (internal/modules/cjs/loader.js:1081:27)
    at exports.importModuleDynamicallyCallback (internal/process/esm_loader.js:37:14)
    at imp (G:\VIZUA\github\node-onezip\node_modules\supertape\lib\super-import.js:20:5)
    at module.exports (G:\VIZUA\github\node-onezip\node_modules\try-to-catch\lib\try-to-catch.js:7:29)
    at module.exports (G:\VIZUA\github\node-onezip\node_modules\supertape\lib\super-import.js:6:31)
    at G:\VIZUA\github\node-onezip\node_modules\supertape\bin\supertape.js:62:15
(node:25964) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on u
nhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 5)
(node:25964) UnhandledPromiseRejectionWarning: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only file and data URLs are supported by the default ESM loader
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:727:11)
    at Loader.resolve (internal/modules/esm/loader.js:97:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:243:28)
    at Loader.import (internal/modules/esm/loader.js:178:28)
    at importModuleDynamically (internal/modules/cjs/loader.js:1081:27)
    at exports.importModuleDynamicallyCallback (internal/process/esm_loader.js:37:14)
    at imp (G:\VIZUA\github\node-onezip\node_modules\supertape\lib\super-import.js:20:5)
    at module.exports (G:\VIZUA\github\node-onezip\node_modules\try-to-catch\lib\try-to-catch.js:7:29)
    at module.exports (G:\VIZUA\github\node-onezip\node_modules\supertape\lib\super-import.js:6:31)
    at G:\VIZUA\github\node-onezip\node_modules\supertape\bin\supertape.js:62:15
(node:25964) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on u
nhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 6)
  1. I tried to implement the test anyway and run it with node test\extract.js but here is the thing:
test('onezip: extract and remove zip (ensure zip file has been released by yaulz)', async (t) => {
    const to = tmp();
    const fixture = join(__dirname, 'fixture');
    const source = join(fixture, 'onezip.txt.zip');
    const from   = join(fixture, 'onezip.txt.cpy.zip');

    copyFileSync(source, from);

    const extracter = extract(from, to);

    await once(extracter, 'end');

    const pathUnpacked = join(to, 'onezip.txt');

    readFileSync(pathUnpacked);

    unlinkSync(pathUnpacked);

    rmdirSync(to);

    unlinkSync(from);

    let fromExists = true;
    try
    {
        statSync(from);
    }
    catch(error)
    {
        fromExists = false;
    }

    t.false(fromExists, 'source zip file has been deleted (zip file released afetr extraction)');
    t.end();
})

I ran that test in debug with a breakpoint on statSync with autoClose=false:

  • unlinkSync(from) never fails even if the file cannot be removed.
  • The from file remains on the filesystem until the node process instance running the test ends.
  • However statSync(from) intended to check the file existence throws error, like the file has already been deleted, but it's not: it's still on the filesystem until the test run ends.

With autoClose=true the from file is really removed from the filesystem just after the unlinkSync(from); instruction.

This is really Windows filesystem weird stuff. You may not be able to reproduced #4 because the zip file is actually removed as soon as your node program ends. For me that's not an option because the unzip and zip file remove occurs in a service that never ends.

@coderaiser coderaiser merged commit abd915d into coderaiser:master Jun 24, 2020
@coderaiser
Copy link
Owner

Thank you 🙂, landed in v4.2.1 🎉

@khelkun khelkun changed the title Fix #2 non closed zip file on windows: use yaulz autoClose=true option Fix #4 non closed zip file on windows: use yaulz autoClose=true option Jun 25, 2020
@khelkun khelkun changed the title Fix #4 non closed zip file on windows: use yaulz autoClose=true option Fix non closed zip file on windows: use yaulz autoClose=true option Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants