NestJS crashes when AWS-SDK throws any type of Error

[LuisOfCourse]

Describe the bug I have got the same problem but with every call that I purposely try to fail:

• wrong key when searching for an object
• wrong credentials...

I was trying to catch the error when a wrong key was inserted (getObject) but I noticed that everything that throws this repo is crashing the entire server.

Expected Behavior The error should be catched by nestjs automatically like other errors in other packages (typeorm ex.). No custom code is needed to catch the errors in nestjs.

Current Behavior All errors thrown by the package crashes the instance of nestjs.

Reproductio Steps: Below the minimal code to reproduce the issue.

controller @Get('test/:test') async getFiles(@Param() { test }, @Res() res: Response) { const file = await this.filesService.getFiles(test); file.createReadStream().pipe(res) }

service async getFiles(test) { return await this.fileAwsS3Service.downloadS3( 'test/' + test + 'file.txt', ); }

aws s3 service

downloadS3(filePath: string) {
  const s3 = this.getS3();
  const params = {
    Bucket: this.configService.get('aws.s3.bucket'),
    Key: filePath,
  };
  return s3.getObject(params);
}

getS3() {
  return new S3({
    region: this.configService.get('aws.s3.region'),
    accessKeyId: this.configService.get('aws.s3.accessKeyID'),
    secretAccessKey: this.configService.get('aws.s3.secret'),
  });
}

If I try to purposely mess up the url/key I get 'key doesn't exists' but the application crashes. This example is the clean code with no try catch nor handlers.

Ill share more examples on test that I made to catch the errors. Like try/catch and eventHandlers.

SDK version used: 2.1151.0 Environment detail: Windows OS, node.js v16.13.2, nestjs 8.2.4

REPOST: Originally posted by @LuisOfCourse in https://github.com/aws/aws-sdk-js/issues/3846#issuecomment-1152930866

[ajredniwja]

Hi @LuisOfCourse, thanks for opening this issue would you be able to give me some more information mentioned in the issue template

[md-shah]

@LuisOfCourse It seems like you don't have a Common Exception Handler in NestJS. If yes, Can you please post it here?

[LuisOfCourse]

@ajredniwja Updated with the additional data you required.

@md-shah Exception Handler in this case does exists and it is the default one of nestjs.

... Out of the box, this action is performed by a built-in global exception filter, which handles exceptions of type HttpException (and subclasses of it). When an exception is unrecognized (is neither HttpException nor a class that inherits from HttpException), the built-in exception filter generates the following default JSON response ...

If I throw an error like this: @Get('test/:test') async getFiles(@Param() { test }) { throw new Error() } The application does not crash, instead nestjs automatically catches it!

[md-shah]

Hi @LuisOfCourse,

I tried to reproduce the same on a NestJS project using Fastify under the hood and without a custom exception handler.

One thing is that. the project didn't crash (At least I couldn't reproduce the same scenario where the app crashes), But the error is not getting thrown properly. But I was able to handle it by converting the function to promise.

Code without callback/promise

downloadS3(filePath: string) {
    const s3 = this.getS3();
    const params = {
      Bucket: this.config.S3.imagesBucket,
      Key: filePath,
    };
    return s3.getObject(params);
  }
  

Response for the same

  [Nest] 38979  - 14/06/2022, 13:59:37   ERROR [ExceptionsHandler] Converting circular structure to JSON
    --> starting at object with constructor 'Request'
    |     property 'response' -> object with constructor 'Response'
    --- property 'request' closes the circle
TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Request'
    |     property 'response' -> object with constructor 'Response'
    --- property 'request' closes the circle
    at JSON.stringify (<anonymous>)
    at serialize (/home/USER/node_modules/fastify/lib/reply.js:707:15)
    at preserializeHookEnd (/home/USER/node_modules/fastify/lib/reply.js:371:15)
    at preserializeHook (/home/USER/node_modules/fastify/lib/reply.js:356:5)
    at _Reply.Reply.send (/home/USER/node_modules/fastify/lib/reply.js:188:7)
    at FastifyAdapter.reply (/home/USER/node_modules/@nestjs/platform-fastify/adapters/fastify-adapter.js:49:29)
    at RouterResponseController.apply (/home/USER/node_modules/@nestjs/core/router/router-response-controller.js:14:36)
    at /home/USER/node_modules/@nestjs/core/router/router-execution-context.js:175:48
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at /home/USER/node_modules/@nestjs/core/router/router-execution-context.js:47:13

{
    "statusCode": 500,
    "message": "Internal server error"
}

The app didn't crash in the above scenario. But when I converted to promise, it worked and got an expected response.

Code with promise

  downloadS3(filePath: string) {
    const s3 = this.getS3();
    const params = {
      Bucket: this.config.S3.imagesBucket,
      Key: filePath,
    };
    return s3.getObject(params).promise();
  }

Response for above code

{
    "statusCode": 404,
    "message": "The specified key does not exist."
}

Tl;dr - Add .promise() and was working as expected.

[LuisOfCourse]

@md-shah thank you for your quick response. Yes, I can confirm. By using the promise you wont face this problem anymore. But this force me to use the promise for each transaction I want to do (with this package). With the method I used above I'm "streaming the file stream" of aws. So I send packeges as they arrives and this means less memory usage. In your case I need to have the complete file before streaming it back to the client. Anyway nestjs won't crash, but won't accept any other call either.

So can you confirm that the this package blocks every other api transaction when error is thrown (and promise is not used)?

[md-shah]

@LuisOfCourse This looks interesting. Yes, the issue exists with the stream and it kill the process as well.

[LuisOfCourse]

The full error is reported here:

C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\services\s3.js:711
      resp.error = AWS.util.error(new Error(), {
                                  ^
NoSuchKey: The specified key does not exist.
    at Request.extractError (C:\Users\User/Documents\Projects\test\test-api\node_modules\aws-sdk\lib\services\s3.js:711:35)
    at Request.callListeners (C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\sequential_executor.js:106:20)
    at Request.emit (C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\sequential_executor.js:78:10)
    at Request.emit (C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\request.js:686:14)
    at Request.transition (C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\request.js:22:10)
    at AcceptorStateMachine.runTo (C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\state_machine.js:14:12)
    at C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\state_machine.js:26:10
    at Request.<anonymous> (C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\request.js:38:9)
    at Request.<anonymous> (C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\request.js:688:12)
    at Request.callListeners (C:\Users\USER\Documents\Projects\test\test-api\node_modules\aws-sdk\lib\sequential_executor.js:116:18)`

[gisioraelvis]

@ajredniwja or @md-shah Any updates on work progress towards resolving this issue?

[Burgov]

I have the same issue. Had to convert

    asset
      .createReadStream()
      .pipe(res);

into

    asset
      .createReadStream()
      .on('error', function () {
        res.sendStatus(404);
      })
      .pipe(res);

to prevent my app from crashing and restarting, but it's a bit annoying that it is necessary.

[nikolaevn]

@Burgov it looks like a bug, see #4333

[toume]

• 1 and it is the same with v3