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

[BUG] Pipeline stream unexpectly being closed when using extract #321

Closed
jeferson-sb opened this issue Aug 9, 2022 · 6 comments · Fixed by #332
Closed

[BUG] Pipeline stream unexpectly being closed when using extract #321

jeferson-sb opened this issue Aug 9, 2022 · 6 comments · Fixed by #332

Comments

@jeferson-sb
Copy link

jeferson-sb commented Aug 9, 2022

What / Why

When you attempt to extract a tarball through streams sometimes it will be closed with an error: [ERR_STREAM_PREMATURE_CLOSE]: Premature close, although the files are successfully extracted. No Errors Codes are shown in the process.

It seems to happen only on newer versions of Node > v18
Looking at the spec it seems now that streams that emit a close event before a end event trigger this error, so worth taking a look on that.

When

  • n/a

Where

  • n/a

How

Current Behavior

  • It throws an ERR_STREAM_PREMATURE_CLOSE Error when trying to extract

Steps to Reproduce

$ nvm install 18.7.0
$ npm install tar got
$ mkdir output
import stream from 'node:stream';
import { promisify } from 'util';
import tar from 'tar';
import got from 'got';

const pipeline = promisify(stream.pipeline);

pipeline(
  got.stream('https://codeload.github.com/npm/node-tar/tar.gz/main'),
  tar.extract({ cwd: './output' }, ['node-tar-main/lib'])
)

Expected Behavior

  • It should extract the content of the archive without any issues

References

  • Related to Next.js Issue #39321
kodiakhq bot pushed a commit to vercel/next.js that referenced this issue Sep 5, 2022
…0182)

`create-next-app` currently cannot extract examples/git repos in Node 18+, because of an issue with `node-tar` (isaacs/node-tar#321). The files are extracted, but an error is thrown that the stream has been prematurely closed. To prevent `create-next-app` from not being able to finish, ~for now, we can swallow this error, and hopefully `node-tar` will be patched soon.~
we can save the tar in the `tmp` folder and extract it from there as suggested: #40182 (review)

I cannot reproduce this on earlier Node.js versions. More context: #39321 (comment)

Fixes #39321



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples)
atilafassina pushed a commit to atilafassina/next.js that referenced this issue Sep 5, 2022
…rcel#40182)

`create-next-app` currently cannot extract examples/git repos in Node 18+, because of an issue with `node-tar` (isaacs/node-tar#321). The files are extracted, but an error is thrown that the stream has been prematurely closed. To prevent `create-next-app` from not being able to finish, ~for now, we can swallow this error, and hopefully `node-tar` will be patched soon.~
we can save the tar in the `tmp` folder and extract it from there as suggested: vercel#40182 (review)

I cannot reproduce this on earlier Node.js versions. More context: vercel#39321 (comment)

Fixes vercel#39321



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples)
@webark
Copy link
Contributor

webark commented Oct 4, 2022

does this mean instead of
https://github.com/npm/node-tar/blob/9d71c5673683d6309c75e6a85e78fa285dbe9a2d/lib/parse.js#L85-L94
you would do

    this.on('end', () => setTimeout(() => this.emit('close')))
    
    if (opt.ondone) {
      this.on(DONE, opt.ondone)
    } else {
      this.on(DONE, _ => {
        this.emit('prefinish')
        this.emit('finish')
        this.emit('end')
      })
    }

essentially? @jeferson-sb @lukekarrys

that seems on odd addition :/

@jeferson-sb
Copy link
Author

@webark yeah looks like it is related to that one line

@webark
Copy link
Contributor

webark commented Oct 5, 2022

meant to say "odd" addition. I was going to try to add a test case.

@webark
Copy link
Contributor

webark commented Oct 5, 2022

I tried adding this test

t.test('ensure an open stream is not prematuraly closed', t => {
  const file = path.resolve(tars, 'body-byte-counts.tar')
  const dir = path.resolve(extractdir, 'basic-with-stream')

  t.beforeEach(async () => {
    await rimraf(dir)
    await mkdirp(dir)
  })

  const check = async t => {
    t.equal(fs.lstatSync(path.resolve(dir, '1024-bytes.txt')).size, 1024)
    await rimraf(dir)
    t.end()
  }

  t.test('async promisey', t => {
    return pipeline(
      fs.createReadStream(file),
      x({ cwd: dir }, ['1024-bytes.txt'])
    ).then(_ => check(t))
  })

  t.end()
})

But it is still passing without any modifications to the code 🤔😔 Any ideas?

webark added a commit to webark/node-tar that referenced this issue Oct 6, 2022
webark added a commit to webark/node-tar that referenced this issue Oct 6, 2022
webark added a commit to webark/node-tar that referenced this issue Oct 13, 2022
@timd73
Copy link

timd73 commented Oct 25, 2022

I'm experiencing this.
Is there a workaround or an expected fix?

@webark
Copy link
Contributor

webark commented Oct 25, 2022

I opened up the PR #332 which fixes the issue. I'm using my fork in a project and it is working. Hopefully it will get merged and released soon.

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 a pull request may close this issue.

3 participants