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
uploadProgress
gets fired after cancelation
#2084
Comments
Can you add Line 1161 in d4c2913
Line 1154 in d4c2913
|
Added console.logs there and the conclusions are:
|
BTW. There is no way to remove |
I think you've just found an edge case. This is what's happening under the hood:
This is valid (and expected) Node.js socket behavior. Probably we need to replace Line 468 in 461b3d4
with if (this._request.destroyed) { Can you confirm if this works?
No, not yet. Please open a separate issue about this. Out of curiosity, what's your use case? |
uploadProgress
gets fired after cancelation
No, unfortunately, that doesn't help. I added a
I'm building a library that will be a part of a Node CLI and an Electron desktop app that will be used by our customers to upload files to our servers. It should support file chunking and its parallel upload with the possibility to pause and resume the transfer. Retries of course too. It should also allow immediate aborting of the whole transfer (with some progress loss) but still with the possibility to resume it in some close undefined future.
|
Well that's quite a comprehensive description 😅 I meant what was the use case of |
Heh. I was in a hurry to catch the weekend and didn't get your question right :) I added a workaround exactly as in this code snippet here #2087 (comment) with an additional comment in the code but still would be great not to have this extra event after the promise was canceled :) |
I tried to reproduce this with a test but the below code fires the event only once so it behaves correctly. I guess it is because there are only two events in total in this unit test or there are some mocks or other mechanisms in place? Posting the code maybe it will be useful for you to do some modifications to reproduce the issue: test('upload progress - should not fire events after cancelation', withServer, async (t, server, got) => {
server.post('/', uploadEndpoint);
const events: Progress[] = [];
const ongoingRequestPromise = got.post({body: file});
let hasCanceled = false;
void ongoingRequestPromise.on('uploadProgress', (event: Progress) => {
console.log("event");
if (hasCanceled) {
events.push(event);
console.log("event pushed");
} else {
console.log("before cancel");
ongoingRequestPromise.cancel();
console.log("after cancel");
}
});
try {
await ongoingRequestPromise;
} catch (error) {
if (error instanceof RequestError && error.code === 'ERR_CANCELED') {
hasCanceled = true;
console.log("hasCanceled = true");
}
}
t.true(hasCanceled);
t.is(events.length, 0);
}); |
Ah, the progress bar, you're right! I'll fix this soon. |
This should be fixed now in |
Sorry for this late response. I removed a workaround from my code and tried to reproduce this issue with no luck so looks like it has been successfully fixed 🎊 |
Describe the bug
Actual behavior
There is always one last progress (at least
uploadProgress
) event fired after the promise was canceled.Expected behavior
After the promise is canceled and the promise rejects with
ERR_CANCELED
there shouldn't be any more progress events fired.Code to reproduce
Checklist
The text was updated successfully, but these errors were encountered: