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

Question: when not to use pump? #52

Open
DenisFrezzato opened this issue Feb 25, 2020 · 6 comments
Open

Question: when not to use pump? #52

DenisFrezzato opened this issue Feb 25, 2020 · 6 comments

Comments

@DenisFrezzato
Copy link

Consider this example.

As soon as stream data have been collected, the HTTP connection is closed just before someAsyncStuff is invoked. Refactoring pump(req, busboy... to req.pipe(busboy) fixes the issue. It's also my understanding that using pump is a best practice for piping streams (for error handling and some clean-up), but in this case it introduces an issue. I would like to understand why this example is not working as I was expecting.

import * as Busboy from 'busboy'
import * as express from 'express'
import * as pump from 'pump'
import concat = require('concat-stream')

const app = express()

app.post('/', (req, res) => {
  const busboy = new Busboy({ headers: req.headers })

  busboy.on('file', (_, file) => {
    pump(
      file,
      concat(buffer =>
        someAsyncStuff(buffer.toString())
          .then(length => res.send({ length }))
          .catch(err => res.status(500).send(err.message))
      ),
      err => {
        if (err) res.status(500).send(err.message)
      }
    )
  })

  pump(req, busboy, err => {
    if (err) res.status(500).send(err.message)
  })
})

function someAsyncStuff(s: string): Promise<number> {
  return new Promise(resolve => setTimeout(() => resolve(s.length), 1))
}

app.listen('3000')
@mafintosh
Copy link
Owner

which error handler is being triggered and what's the error?

@DenisFrezzato
Copy link
Author

No error at all. The HTTP connection is just closed, but someAsyncStuff goes on with its execution.

@mafintosh
Copy link
Owner

pump doesn't do anything magic, don't know enough about the modules used to know if they do. If you share a runnable end-to-end gist, I'm happy to try it.

@DenisFrezzato
Copy link
Author

Do you have any playground to suggest with which I can make this example with Express work? The code I posted is a complete example, you could just copy-paste it in a .ts file and run it and try a POST request with some file in the payload.

@mafintosh
Copy link
Owner

A gist with a thing I can run with npm install && node server.js and a curl command that reproduces the issue you are seeing (or something other than curl).

@DenisFrezzato
Copy link
Author

DenisFrezzato commented Feb 25, 2020

Here it is https://github.com/DenisFrezzato/question-pump

EDIT: I also tried with pipeline and the result is the same (but I guess this is not surprising).

@DenisFrezzato DenisFrezzato changed the title Question: when not use pump? Question: when not to use pump? Feb 20, 2021
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

No branches or pull requests

2 participants