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

TypeError: file.destroy is not a function #259

Closed
alexconstantin opened this issue Nov 15, 2020 · 3 comments
Closed

TypeError: file.destroy is not a function #259

alexconstantin opened this issue Nov 15, 2020 · 3 comments

Comments

@alexconstantin
Copy link

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
Copy link
Collaborator

RomanBurunkov commented Sep 28, 2023

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
Copy link
Collaborator

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 added a commit that referenced this issue Oct 3, 2023
Fix issue #259 & some rework and formatting for utilities tests.
@RomanBurunkov
Copy link
Collaborator

Fixed with #358

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