nodejs-storage icon indicating copy to clipboard operation
nodejs-storage copied to clipboard

Interrupted download stream does not retry correctly after some bytes are sent

Open ddelgrosso1 opened this issue 2 years ago • 1 comments

Creating an issue to track this. Currently, if the request body stream of a download is interrupted / forcefully closed the library is not correctly retrying. After some initial investigation it appears that this may be related to a problem in node-fetch as outlined here. Currently investigating possible solutions as the version in which this was fixed for node-fetch requires the ability to utilize ESM.

ddelgrosso1 avatar Mar 23 '22 13:03 ddelgrosso1

Tagged as external because this is an issue with node-fetch / teeny-request

shaffeeullah avatar Aug 10 '22 14:08 shaffeeullah

Updating this as a self reminder of sorts: this appears to have been fixed in node-fetch v3.1.1 . It does not appear that the fix was ever back ported to 2.x.

ddelgrosso1 avatar Jun 20 '23 18:06 ddelgrosso1

Possibly related as well: https://github.com/node-fetch/node-fetch/pull/1064. This appears to have been backported and released with 2.6.9. We moved to an async iterator with this PR: https://github.com/googleapis/nodejs-storage/pull/1925 this was around the same time we noticed this bug.

ddelgrosso1 avatar Jun 22 '23 20:06 ddelgrosso1

To my last comment, I was able to verify hanging behavior with 2.6.7 but get a correct error with 2.6.9 of node-fetch.

ddelgrosso1 avatar Jun 22 '23 21:06 ddelgrosso1

import express from 'express';
import fetch from 'node-fetch';

const app = express();
const port = 5000;
let count = 0;
app.get('/', (req, res) => {
    setInterval(() => {
        count++;
        res.write('hello');
        if (count >= 10) {
            res.socket?.destroy();
        }
    }, 1000);
});
app.listen(port, () => console.log(`Running on port ${port}`));

try {
    const response = await fetch('http://localhost:5000', {
        method: 'GET'
    });
    for await (const chunk of response.body) {
        console.log(chunk);
    }
} catch (e) {
    console.error(e);
}
console.log('Finished async iterator');

With node-fetch v2.6.7 this will reliably recreate unresponsiveness. With 2.6.9 the error is thrown / caught correctly.

ddelgrosso1 avatar Jun 23 '23 14:06 ddelgrosso1

Duplicate of: https://github.com/googleapis/nodejs-storage/issues/2193. Fixing the node-fetch issue will fix this problem.

danielduhh avatar Jan 31 '24 21:01 danielduhh