chore: update to aws-sdk 3.4.1 #7726
Conversation
| @@ -21,12 +21,12 @@ import { | |||
| getAmplifyUserAgent, | |||
| } from '@aws-amplify/core'; | |||
| import { | |||
| EventsBatch, | |||
There was a problem hiding this comment.
No need to import from @aws-sdk/client-pinpoint/models since it is exported from the root: https://github.com/aws/aws-sdk-js-v3/blob/master/clients/client-pinpoint/index.ts#L115
| @@ -350,7 +370,6 @@ export class AWSS3ProviderManagedUpload { | |||
| ...localTestingConfig, | |||
| requestHandler: new AxiosHttpHandler({}, emitter), | |||
| customUserAgent: getAmplifyUserAgent(), | |||
| urlParser: parseUrl, | |||
There was a problem hiding this comment.
Per conversation with SDK team, there is no need to explicitly pass urlParser because S3Client already knows what parser needs to be used based on the environment. Based on initial testing, this change doesn't seem to have any impact on functionality.
This was removed because @aws-sdk/url-parser-node has a breaking change for React Native.
| @@ -16,14 +16,14 @@ import { HttpHandler, HttpRequest, HttpResponse } from '@aws-sdk/protocol-http'; | |||
| import { buildQueryString } from '@aws-sdk/querystring-builder'; | |||
| import axios, { AxiosRequestConfig, Method } from 'axios'; | |||
| import { ConsoleLogger as Logger } from '@aws-amplify/core'; | |||
| import { BrowserHttpOptions } from '@aws-sdk/fetch-http-handler'; | |||
| import { FetchHttpHandlerOptions } from '@aws-sdk/fetch-http-handler'; | |||
There was a problem hiding this comment.
This was renamed in rc.8: aws/aws-sdk-js-v3#1697
| // @aws-sdk/client-s3 seems to be ignoring the `ContentType` parameter, so we | ||
| // are explicitly adding it via middleware. | ||
| // https://github.com/aws/aws-sdk-js-v3/issues/2000 | ||
| s3.middlewareStack.add( | ||
| next => (args: any) => { | ||
| if ( | ||
| this.params.ContentType && | ||
| args && | ||
| args.request && | ||
| args.request.headers | ||
| ) { | ||
| args.request.headers['Content-Type'] = this.params.ContentType; | ||
| } | ||
| return next(args); | ||
| }, | ||
| { | ||
| step: 'build', | ||
| } | ||
| ); |
There was a problem hiding this comment.
Comment is pretty self explanatory, but there appears to be an issue with the SDK where it is removing ContentType when creating CreateMultipartUploadCommand, which causes multi-part uploads to fail. For now, we can get around this by using middleware, and we can revisit this in future SDK upgrades.
We now have multiple tests in CircleCI for React & React Native (iOS & Android) to validate multi-part uploads.
Codecov Report
@@ Coverage Diff @@
## main #7726 +/- ##
==========================================
- Coverage 74.10% 74.07% -0.03%
==========================================
Files 214 214
Lines 13405 13410 +5
Branches 2626 2628 +2
==========================================
Hits 9934 9934
- Misses 3272 3277 +5
Partials 199 199
Continue to review full report at Codecov.
|
|
Thanks @amhinson!! 🌮 🎉 |
|
Ty @amhinson 👍 👍 |
|
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
Issue #, if available:
Description of changes:
Reference: https://github.com/aws/aws-sdk-js-v3/releases
Successful CircleCI build with integration tests: https://app.circleci.com/pipelines/github/aws-amplify/amplify-js/7181/workflows/171bfbee-0e77-43ff-9478-3d20ae5176bd
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.