aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.41k stars 2.12k forks source link

Storage.put resumable and useAccelerateEndpoint options do not work together #9143

Closed mkrn closed 2 years ago

mkrn commented 2 years ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

Storage

Amplify Categories

storage

Environment information

``` # Put output below this line System: OS: macOS 11.6 CPU: (8) x64 Apple M1 Memory: 28.05 MB / 16.00 GB Shell: 3.2.57 - /bin/bash Binaries: Node: 12.16.1 - ~/.nvm/versions/node/v12.16.1/bin/node Yarn: 1.22.11 - ~/Sites/eventlive-universal/node_modules/.bin/yarn npm: 6.13.4 - ~/.nvm/versions/node/v12.16.1/bin/npm Watchman: 4.9.0 - /usr/local/bin/watchman Browsers: Chrome: 95.0.4638.54 Firefox: 89.0.1 Safari: 15.0 npmPackages: @babel/cli: ^7.12.10 => 7.14.8 @babel/core: ~7.9.0 => 7.9.6 (7.9.0, 7.15.0, 7.12.3) @babel/plugin-proposal-class-properties: ^7.12.1 => 7.14.5 (7.12.13, 7.12.1) @babel/preset-env: ^7.12.11 => 7.15.0 (7.12.17, 7.15.8, 7.12.1) @expo/webpack-config: ~0.12.63 => 0.12.82 @react-native-async-storage/async-storage: ~1.15.0 => 1.15.5 @react-native-community/datetimepicker: 3.5.2 => 3.5.2 @react-native-community/masked-view: 0.1.10 => 0.1.10 @react-native-community/netinfo: 6.0.0 => 6.0.0 @react-native-community/segmented-control: ^2.2.2 => 2.2.2 @react-native-picker/picker: 1.16.1 => 1.16.1 (1.16.5) @react-navigation/native: ^5.8.10 => 5.9.6 @react-navigation/stack: ^5.12.8 => 5.14.7 @segment/analytics-react-native: ^1.5.0 => 1.5.0 @sentry/react-native: 2.6.0 => 2.6.0 (2.6.2) @types/react: ~16.9.35 => 16.9.56 (17.0.16) @types/react-native: ~0.63.2 => 0.63.53 HelloWorld: 0.0.1 RNBackgroundExample: 0.0.1 add: ^2.0.6 => 2.0.6 amazon-cognito-identity-js: ^4.5.6 => 4.6.3 (5.2.2) aws-amplify: 4.3.4 => 4.3.4 babel-jest: ^26.6.0 => 26.6.3 babel-plugin-inline-import: ^3.0.0 => 3.0.0 babel-plugin-lodash: ^3.3.4 => 3.3.4 branch-sdk: ^2.58.2 => 2.58.2 customize-cra: ^1.0.0 => 1.0.0 expo: ^42.0.0 => 42.0.3 expo-application: ~3.2.0 => 3.2.0 expo-clipboard: ~1.1.0 => 1.1.0 expo-constants: ~11.0.1 => 11.0.1 expo-device: ~3.3.0 => 3.3.0 expo-document-picker: ~9.2.4 => 9.2.4 expo-file-system: ~11.1.3 => 11.1.3 expo-font: ~9.2.1 => 9.2.1 expo-image-picker: ~10.2.2 => 10.2.2 expo-keep-awake: ~9.2.0 => 9.2.0 expo-linking: ~2.3.1 => 2.3.1 expo-localization: ~10.2.0 => 10.2.0 expo-media-library: ~12.1.2 => 12.1.2 expo-permissions: ~12.1.1 => 12.1.1 expo-sharing: ~9.2.1 => 9.2.1 expo-splash-screen: ~0.11.2 => 0.11.2 expo-status-bar: ~1.0.4 => 1.0.4 expo-updates: ^0.8.5 => 0.8.5 expo-video-thumbnails: ~5.2.1 => 5.2.1 expo-web-browser: ~9.2.0 => 9.2.0 formik: ^2.2.6 => 2.2.9 hermes-inspector-msggen: 1.0.0 i18n-js: ^3.8.0 => 3.8.0 install: ^0.13.0 => 0.13.0 jest: 26.6.0 => 26.6.0 moment: ^2.29.1 => 2.29.1 moment-timezone: ^0.5.32 => 0.5.33 native-base: 2.15.2 => 2.15.2 papaparse: ^5.3.0 => 5.3.1 patch-package: ^6.2.2 => 6.4.7 react: 16.13.1 => 16.13.1 (0.14.10) react-animated: 0.1.0 react-app-rewired: ^2.1.7 => 2.1.8 react-art: ^17.0.1 => 17.0.2 react-async-hook: ^3.6.2 => 3.6.2 react-datepicker: ^3.3.0 => 3.8.0 react-device-detect: ^1.14.0 => 1.17.0 react-dom: 16.13.1 => 16.13.1 (0.14.10) react-dropzone: ^11.2.4 => 11.3.4 react-native: 0.63.4 => 0.63.4 react-native-background-upload: ^6.2.0 => 6.4.0 react-native-branch: ^5.0.0 => 5.0.3 react-native-device-info: ^8.3.1 => 8.3.1 react-native-dropdownalert: ^4.3.0 => 4.3.0 react-native-gesture-handler: ~1.10.2 => 1.10.3 react-native-iap: ^7.3.0 => 7.4.0 react-native-keyboard-aware-scroll-view: ^0.9.3 => 0.9.4 react-native-keyevent: ^0.2.8 => 0.2.8 react-native-modal-datetime-picker: ^10.0.0 => 10.2.0 react-native-permissions: ^3.0.0 => 3.0.5 react-native-progress: ^4.1.2 => 4.1.2 react-native-reanimated: ^2.2.0 => 2.2.0 react-native-responsive-dimensions: ^3.1.1 => 3.1.1 react-native-safe-area-context: 3.2.0 => 3.2.0 react-native-safe-area-view: ^1.1.1 => 1.1.1 react-native-screens: ~3.4.0 => 3.4.0 react-native-super-grid: 4.1.3 => 4.1.3 react-native-svg: 12.1.1 => 12.1.1 react-native-svg-transformer: ^0.14.3 => 0.14.3 react-native-switch: ^2.0.0 => 2.0.0 react-native-toast-banner: ^1.0.0 => 1.0.0 react-native-unimodules: ~0.14.5 => 0.14.6 react-native-web: ^0.17.1 => 0.17.1 react-qr-code: ^2.0.1 => 2.0.1 react-scripts: ^4.0.1 => 4.0.3 react-test-renderer: ~16.13.1 => 16.13.1 sentry-expo: ^4.0.0 => 4.0.1 typescript: ~4.0.0 => 4.0.8 use-clipboard-copy: ^0.2.0 => 0.2.0 use-force-update: ^1.0.7 => 1.0.7 uuidv4: ^6.2.6 => 6.2.11 webpack-bundle-analyzer: ^4.3.0 => 4.4.2 webpack-visualizer-plugin: ^0.1.11 => 0.1.11 yarn: ^1.22.10 => 1.22.11 yup: ^0.32.8 => 0.32.9 npmGlobalPackages: aws-cdk: 1.119.0 expo-cli: 4.7.2 npm: 6.13.4 ```

Describe the bug

When using resumable: true option together with useAccelerateEndpoint: true the latter will not apply. It would still use non-accelerated endpoint

This is likely due to how the new S3 Client is created in Put if resumable is enabled. It is not passing the useAccelerateEndpoint in the options.

It is because both features were developed around the same time, they were not tested together.

https://github.com/aws-amplify/amplify-js/blob/f8faa1c86f0707b4ac3b8a3fada15d078712cabb/packages/storage/src/providers/AWSS3Provider.ts#L825

Expected behavior

When using the new useAccelerateEndpoint: true I expect the accelerated bucket endpoint would be used.

Reproduction steps

Enable Accelerated uploads on a bucket Call Storage.put request with bothresumable: true, and useAccelerateEndpoint: true Open network preferences in browser while uploading Observe requests hostnames

Code Snippet

// Put your code below this line.

Log output

``` // Put your logs below this line ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

jamesaucode commented 2 years ago

Thanks for opening the issue. I submitted a PR that should fix this problem

mkrn commented 2 years ago

Thank you, it looks LGTM, when can the new version be expected @jamesaucode ?

github-actions[bot] commented 1 year ago

This issue 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 *-help channels or Discussions for those types of questions.