richardgirges / express-fileupload

Simple express file upload middleware that wraps around busboy
MIT License
1.52k stars 261 forks source link

TypeError: file.destroy is not a function #259

Closed alexconstantin closed 11 months ago

alexconstantin commented 3 years ago

Hello,

We sometimes get this error:

TypeError: file.destroy is not a function

  | 2020-11-13T13:01:02.147+02:00 | at Timeout.UploadTimer [as _onTimeout] (/app/node_modules/express-fileupload/lib/processMultipart.js:65:12)   | 2020-11-13T13:01:02.147+02:00 | at ontimeout (timers.js:365:14)   | 2020-11-13T13:01:02.147+02:00Copy at tryOnTimeout (timers.js:237:5) | at tryOnTimeout (timers.js:237:5)   | 2020-11-13T13:01:02.147+02:00 | at Timer.listOnTimeout (timers.js:207:5)

We are using fileupload with this settings: app.use(fileUpload({ uploadTimeout: 1000 60 10, useTempFiles : true, tempFileDir : '/tmp/', preserveExtension: true, debug: config.env !== 'prod' }));

Is it something I am missing? Do you know any workarounds until this gets fixed? Thank you!

RomanBurunkov commented 11 months ago

I've tried to reproduce this issue by using the same settings and aborting upload from the client end.

A destroy method is calling from an uploadTimer instance and callback is:

() => {
      file.removeAllListeners('data');
      file.resume();
      // After destroy an error event will be emitted and file clean up will be done.
      file.destroy(new Error(`Upload timeout ${field}->${filename}, bytes:${getFileSize()}`));
}

So according to the error file definitely has removeAllListeners and resume methods, however there is no destroy. Searching through internet shows several cases, where sometimes stream doesn't have destroy method.

Probably was related to the previous bb or node versions.

Suggestion is to add additional check for existence of the destroy method, before calling it and also add debug log if there is no.

Another and probably better idea is to emit error with file.emit, so all cleanups will be also done.

RomanBurunkov commented 11 months ago

Something like this:

      const err = new Error(`Upload timeout for ${field}->${filename}, bytes:${getFileSize()}`);
      return isFunc(file.destroy) ? file.destroy(err) : file.emit('error', err);
RomanBurunkov commented 11 months ago

Fixed with #358