If __removeFile() throws exception after an error occurred, the error will not be emitted/handled

[DrewImm]

Stumbled on 4 occurrences in the code of missing .catch()/.finally().

If __removeFile throws an exception, .then() will not be called, and the cleanup code will not execute.

.then() here should be changed to .finally()

e.g. (this one is from __onTimeout())

return this.__removeFile().then(() => {
    this.__setState(this.__states.FAILED);
    this.emit('timeout');
    reject(new Error('timeout'));
});

Although __removeFile() does not call a promise rejector, that does not preclude it from throwing an exception (calling .catch() instead of .then()). It will also likely lead to bugs if __removeFile() is ever changed to throw on unlink failure.

Not a huge deal but hope this can help reduce some headache troubleshooting down the road!

[hgouveia]

hello @DrewImm , in theory __removeFile shouldnt throw an error, since all the calls we made in there, like the fileStream.close and fs.unlink use the nodejs callback way of thing, i mean callback(err, result) , and each of this calls we are not checking for err , and resolve netherless , i intetionally resolve even if fails since this doesnt mean it wasnt stopped but i agree as extra mesuarment i should change it to finally specially that i now upgraded the lib to node 14, and also i might introduce a new event called "warning" just to notify the user in case it failed to remove, and want to do something about it but without trigger an error

[hgouveia]

Implemented on v2.1.0