mscdex / busboy

A streaming parser for HTML form data for node.js
MIT License
2.84k stars 213 forks source link

ENOENT error after busboy received the file #233

Closed ivancortesromero closed 2 years ago

ivancortesromero commented 4 years ago

On a legacy project, MP4 files are uploaded to a NestJS API with busboy on an EC2 instance. Then, the file is uploaded to an S3 bucket.

When the AWS S3 library tries to load the file, an apparent "random" error is raised sometimes:

 warn
 module: videoservice
 method: saveTostream
 message: Error on Upload Success callback
 { [Error: ENOENT: no such file or directory, open 'file.mp4'] errno: -2, code: 'ENOENT', syscall: 'open', path: 'file.mp4' }

 (node:26886) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory, open 'file.mp4'
 (node:26886) UnhandledPromiseRejectionwarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catc h(). (rejection id: 20)

Here are the snippets:

API upload endpoint

  @Post('upload/:id')
  @ApiOperation({ ... })
  @ApiResponse({ status: 201, type: Video })
  @ApiConsumes('multipart/form-data')
  @ApiImplicitFile({ name: 'file', required: true })
  @ApiCreatedResponse({ type: UploadResponseDto })
  async upload(
    @Decoded() decoded: any,
    @Param('id') videoId: number,
    @Query('fileSize') fileSize :number,
    @Req() req: Request,
    @Res() res: Response,
    @Body() uploadDto: UploadDto,
  ): Promise<any> {
    try {
      const busboy = new Busboy({
        headers: req.headers,
      });

      const data = new Map();
      const writtenFiles = [];
      let upload;

      busboy.on('field', (fieldname, val) => {
        data.set(fieldname, val);
      });

      busboy.on(
        'file',
        async (fieldname, file, filename, encoding, mimetype) => {
          try {
            data.set('filename', filename);
            data.set('mimetype', mimetype);
            data.set('encoding', encoding);

            const filepath = path.join(fieldname + path.extname(filename));
            upload = filepath;

            const writeStream = fs.createWriteStream(filepath);
            file.pipe(writeStream);

            const promise = new Promise((resolve, reject) => {
              file.on('end', () => {
                writeStream.end();
              });
              writeStream.on('finish', resolve);
              writeStream.on('error', reject);
            });

            writtenFiles.push(promise);
          } catch (err) {
            this.logger.error(log('busboy on file', err.message));
            return res.status(HttpStatus.INTERNAL_SERVER_ERROR).send({
              statusCode: HttpStatus.INTERNAL_SERVER_ERROR,
              message: err.message,
            });
          }
        },
      );

      busboy.on('error', err => {
        this.logger.warn(log('busboy on error', err.message));
      });

      busboy.on('finish', async () => {
        await Promise.all(writtenFiles);
        res.status(HttpStatus.CREATED).send('OK');

        await this.bucketService.saveStream( // Next snippet
          fs.createReadStream(upload),
          data.get('filename'),
          data.get('mimetype'),
          data.get('fileSize'),
          +data.get('programId'),
          +data.get('videoId')
        );
        return;
        fs.unlinkSync(upload);
      });

      req.pipe(busboy);
    } catch (err) {
      this.logger.error(log('catch', err.message));
      return res.status(HttpStatus.INTERNAL_SERVER_ERROR).send({
        statusCode: HttpStatus.INTERNAL_SERVER_ERROR,
        message: err.message,
      });
    }
  }

BucketService saveStream method

public async saveStream(
    body: any
    filename: string,
    mimeType: string,
    fileSize: number,
    programId: number,
    videoId: number
  ): Promise<any> {
    try {
      const callback = async (err: any, data: any) => {
        if (err) {
          this.logger.warn(log('Error on Upload Success callback', err));
          throw err; // Here is where the error is raised
        }
      };

      return this.s3
        .upload(
          this.setParams(body, filename, mimeType, programId, videoId),
          (err, data) => callback(err, data),
        )
    } catch (err) {
      this.logger.error(log('Error on S3 Upload', err));
      throw err;
    }
  }

  private setParams(
    file: any,
    filename: string,
    mimeType: string,
    programId: number,
    videoId: number
  ): any {
    return {
      ...awsBucketConfig,
      Key: `${AWS_UPLOAD_DIR}${programId}/${videoId}/${Date.now()}${path.extname(
        filename,
      )}`,
      Body: file,
      ContentType: mimeType,
    };
  }

At some moment I thought perhaps this happens because the name of the temporal file on EC2 is always the same: file.mp4, and when two files are uploaded at the same time, the first one on finishing removes the file (fs.unlinkSync(upload); at the endpoint) leaving the other ongoing process without it, so when tries to upload it, this process won't find it. But it is not true because I performed tests where I ensured the files were uploaded one by one. However, I also ensured the name was always different by changing on the controller:

const filepath = path.join(fieldname + path.extname(filename));

by

const filepath = path.join(Math.floor(Math.random() * 10000) + path.extname(filename));

but the error is still happening. Another weird thing which is happening is that in my machine I can see (ls) the file meanwhile it is uploaded, but in EC2 not.

Facts:

Thank guys.

ivancortesromero commented 4 years ago

I found this issue on async-busboy package which seems related to this error, but I'm using the version 0.7.0, a version greater than the one where it is supposed was resolved.

mscdex commented 4 years ago

Two things I noticed:

  1. The main one being you're listening for 'finish' on the writable file stream instead of 'close'. The latter indicates when the file has actually been closed and would probably be a better signal to wait for.

  2. The following is unnecessary code since file.pipe(writeStream) already does it for you:

  file.on('end', () => {
    writeStream.end();
  });