zemirco / json2csv

Convert json to csv with column titles
http://zemirco.github.io/json2csv
MIT License
2.71k stars 366 forks source link

promise() not listen for 'close' event #559

Open tjpgt opened 2 years ago

tjpgt commented 2 years ago

The promise() function listens for 'finish' and 'error', but dosen't listen for 'close'.

For example, I have a http server, and use json2csv to export a csv. I use toOutput to pipe to http response, then use promise() and wait for the promise. When I start a request and cancel the requset(use postman, for example), the http response emit a 'close' event, but the promise never settle.

Code like this:

const asyncParser = new AsyncParser(opts, transformOpts);
const parsePromise = asyncParser.toOutput(res).promise();          // res is the http response
......
read much data and push to asyncParser.input
......
asyncParser.input.push(null);
await parsePromise  // if 'close' event but no 'finish' or 'error' is emitted on res, it will never resolve or reject

Though I can write my own promise listen for 'close' event instead of using promise(). It would be better if it's added in the promise() function in json2csv.

juanjoDiaz commented 2 years ago

This is an interesting one... Any idea of why is dispatching close but not finish? Is it a bug?

I would say that it should either finish or error.

tjpgt commented 2 years ago

This is an interesting one... Any idea of why is dispatching close but not finish? Is it a bug?

I would say that it should either finish or error.

image

From nodejs api doc: https://nodejs.org/docs/latest-v14.x/api/http.html#http_class_http_serverresponse

So I think if the response is sended normally, it will emit 'finish'. But if not, the request is aborted by user for example, it will emit 'close', and I think it's emitted because the socket emits the 'close' event.

And as for 'stream' , there is: https://nodejs.org/docs/latest-v14.x/api/stream.html#stream_class_stream_writable

image

image