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

How to know when writeStream is completed? #147

Closed
sean256 opened this issue Feb 15, 2019 · 8 comments
Closed

How to know when writeStream is completed? #147

sean256 opened this issue Feb 15, 2019 · 8 comments

Comments

@sean256
Copy link
Contributor

sean256 commented Feb 15, 2019

I need to know when a createWriteStream has fully completed.

I tried on events for end, finish and unpipe and all of them get called immediately and before the write is complete.

const readStream = fs.createReadStream('./test.zip');
const writeStream = client.createWriteStream('/some/remote/place/test.zip');
writeStream.on('end', () => console.log('end'))
		.on('unpipe', () => console.log('unpipe'))
		.on('finish', () => console.log('finish'));
readStream.pipe(writeStream);
@sean256
Copy link
Contributor Author

sean256 commented Feb 15, 2019

I found a way to get a proper event when it's done but requires a code change in webdav-client

in createStream.js -> createWriteStream I added a line to emit to writeStream when request does it's thing.

prepareRequestOptions(requestOptions, options);
    request(requestOptions)
        .then(responseHandlers.handleResponseCode)
        .then(() => {
            writeStream.emit("complete");
        })
        .catch(err => {
            writeStream.emit("error", err);
        });
    return writeStream;

@perry-mitchell
Copy link
Owner

perry-mitchell commented Feb 20, 2019

I'm not sure what "complete" would do.. Aren't the stream events only close, open and ready (WriteSteam)?

That being said, close should do what you need.. I believe axios should close it once it's complete. You can see in a test that there's been problems here.. But not sure exactly what the issue is.

@sean256
Copy link
Contributor Author

sean256 commented Feb 21, 2019

You're right. Here is what I need to know and it looks like one of your tests have the same need.

I need to know when the "WriteStream" is totally and completely done doing its thing, including passing off to request and request completing. My current use case is in AWS Lambda and if you return in the handle function AWS may freeze your lambda until is awaken again even if there is still stuff to do in the event loop!

Besides my use in AWS Lambda I think knowing when it's completed is a good thing to have anyway. I just don't know what the best option is here with the way it's set up.

// here's a snippet of what I'm doing using the proposed `completed`. This is wrapped up in a promise.
const readStream = s3.getObject({ Bucket, Key }).createReadStream().on('error', reject);
const writeStream = webdavClient.createWriteStream(`${remotePath}/${filename}`);
readStream.pipe(writeStream);
writeStream.on('complete', resolve); // resolve promise

@perry-mitchell
Copy link
Owner

I've flagged this as a bug, which I'll hopefully get around to shortly. I'm also happy for someone else to take a look at it!

@bingliu221
Copy link

Same issue here.
I currently emit a new event on the writeStream, like @sean256 did. But I don't think it's a proper way to solve it.

The writeStream should be closed when axios finishes reading from it, but i don't think it's the reason or solution of this issue. Even though the stream is closed properly, which means axios has read all the input data, there is still a chance the request would fail.

@bingliu221
Copy link

I pass the a readable stream as arg data at putFileContents, and unset the Content-Length header.

function putFileContents(filePath, data, options) {
    const headers = {};
    if (!(data instanceof stream.Readable)) {
        headers["Content-Length"] = data.length;
    }
    const putOptions = merge(getPutContentsDefaults(), { headers }, options || {});
    if (putOptions.overwrite === false) {
        putOptions.headers["If-None-Match"] = "*";
    }
    const requestOptions = {
        url: joinURL(options.remoteURL, encodePath(filePath)),
        method: "PUT",
        headers: putOptions.headers,
        data
    };
    prepareRequestOptions(requestOptions, options);
    return request(requestOptions).then(responseHandlers.handleResponseCode);
}
client.putFileContents('dstPath', readableStream, options)

It works for me. I can now know when the uploading is completed.
I'm not familiar with JavaScript, is it an appropriate modification?

@perry-mitchell
Copy link
Owner

@bingliu221 Your solution actually sounds quite logical. There's no way to really know the length of the stream if it's still open, so sending it as a header is pointless.

I would accept a PR for the if (!(data instanceof stream.Readable)) { logic!

@perry-mitchell
Copy link
Owner

Released in 3.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants