aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
3.05k stars 572 forks source link

MIGRATION ISSUE: v2 to v3 promises on s3 object resolving when there's network error #5683

Open niiapa opened 8 months ago

niiapa commented 8 months ago

Pre-Migration Checklist

Which JavaScript Runtime is this issue in?

Browser

AWS Lambda Usage

Describe the Migration Issue

In v3, functions on the S3 object have their promises resolve incorrectly on network errors. Whereas in v2, this behaviour did not happen and a network error would result in the promise rejecting and throwing an error. The behaviour was noticed in the createMultipartUpload and uploadPart functions however, I'm not sure if other functions are affected or the reason for this.

Code Comparison

v2:

// Create s3 object...
s3 = new AWS.S3({
    credentials: awsCredentials,
    region: bucketRegion,
    httpOptions: {
      timeout: 7200000,
    },
    useAccelerateEndpoint: accelerateSupported,
    correctClockSkew: true,
  });
}

// Use s3 object to create multipart upload...
s3.createMultipartUpload(bucketParam).promise()
  .then((s3Data) => {...}
  .catch((e) => {...} // this catches network errors

v3:

// Create s3 object...
s3 = new S3({
  credentials: credentialsObj,
  region: bucketRegion,
  useAccelerateEndpoint: accelerateSupported,
});

// Use s3 object to create multipart upload...
s3.createMultipartUpload(bucketParam)
  .then((s3Data) => {...} // I'm having to manually check for uploadId existence here to see if the request actually completed
  .catch((e) => {...} // this DOES NOT catch network errors

Observed Differences/Errors

v2 functions catch network errors in the catch block whereas the v3 versions don't appear to throw any errors on network errors. Network error being connection disabled or endpoint blocked so request doesn't make it through to AWS

Additional Context

No response

RanVaknin commented 8 months ago

Hi @niiapa,

Can you please give us an example of what a network error might look like?

Thanks, Ran~

niiapa commented 8 months ago

Hi @RanVaknin, This might be hard to do... since if there's a network error there's no response generated. You can simulate a network error on Chrome/Edge by blocking the network requests using Network Request Blocking for the parts.

Example block URL pattern for uploadPart request using accelerate endpoint: *.s3-accelerate.amazonaws.com/*/original?partNumber=*&uploadId=*

This would prevent the request from being completed successfully.

An alternative would be to manually set network to offline during the multipart upload process but getting the timing right with this can be a bit trickier.

RanVaknin commented 1 month ago

Hi @niiapa ,

I have disconnected my machine's access to the internet mid upload to simulate a network error and the behavior between v2 and v3 is the same. With networking errors, both SDKs will reject the promise and the error can be logged from a catch block.

v3 error:

$ node sample.mjs
Multipart upload initiated RAsE.YnaSwdKrFyKIMIEkb5nj48JLpnxEZZMLac1VfDL8OW_EcbgfuEEh2AH7.KAXyY2AxAd.NUkSMF4lwrcCPUXKpw6q47KNo8CCkWLtsxzAeyUFjVeYR28VpLmzWXm
Part 1 uploaded "79b281060d337b9b2b84ccf390adcf74"
Upload failed Error: getaddrinfo ENOTFOUND testbucket-3650.s3.us-east-1.amazonaws.com
    at GetAddrInfoReqWrap.onlookupall [as oncomplete] (node:dns:120:26) {
  errno: -3008,
  code: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'testbucket-3650.s3.us-east-1.amazonaws.com',
  '$metadata': { attempts: 2, totalRetryDelay: 0 }
}

v2 error:

$ node v2sample.js
Multipart upload initiated ohHbEt2.zz022xBNgzuz4SS.3jkNMUbtcE5jEc7kXkpN9C1H093DfrnR0aeDs7gl2zafYJa4aW0hFYgUtW4M0sq.hAa2LGGR8YnXMe0VZuq8ev_YUcLaxdjTySPM.w.v
Part 1 uploaded "79b281060d337b9b2b84ccf390adcf74"
Upload failed UnknownEndpoint: Inaccessible host: `testbucket-3650.s3.amazonaws.com' at port `undefined'. This service may not be available in the `us-east-1' region.
    at Request.ENOTFOUND_ERROR (/Users/rvaknin/test_folder/5683/node_modules/aws-sdk/lib/event_listeners.js:612:46)
    at Request.callListeners (/Users/rvaknin/test_folder/5683/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
    at Request.emit (/Users/rvaknin/test_folder/5683/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (/Users/rvaknin/test_folder/5683/node_modules/aws-sdk/lib/request.js:686:14)
    at error (/Users/rvaknin/test_folder/5683/node_modules/aws-sdk/lib/event_listeners.js:443:22)
    at ClientRequest.<anonymous> (/Users/rvaknin/test_folder/5683/node_modules/aws-sdk/lib/http/node.js:100:9)
    at ClientRequest.emit (node:events:520:28)
    at ClientRequest.emit (node:domain:488:12)
    at TLSSocket.socketErrorListener (node:_http_client:495:9)
    at TLSSocket.emit (node:events:520:28) {
  code: 'UnknownEndpoint',
  region: 'us-east-1',
  hostname: 'testbucket-3650.s3.amazonaws.com',
  retryable: true,
  originalError: Error: getaddrinfo ENOTFOUND testbucket-3650.s3.amazonaws.com
      at GetAddrInfoReqWrap.onlookupall [as oncomplete] (node:dns:120:26) {
    errno: -3008,
    code: 'NetworkingError',
    syscall: 'getaddrinfo',
    hostname: 'testbucket-3650.s3.amazonaws.com',
    region: 'us-east-1',
    retryable: true,
    time: 2024-08-05T21:51:03.545Z
  },
  time: 2024-08-05T21:51:03.545Z
}

Perhaps use try / catch instead of .then().catch()

Thanks, Ran~

github-actions[bot] commented 1 month ago

This issue has not received a response in 1 week. If you still think there is a problem, please leave a comment to avoid the issue from automatically closing.