-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Storage.get no longer working after V5.3.23 update #13853
Comments
Thanks for reporting this issue! We will look into this. |
It looks like the Update: this issue is actually reproducible with both Amplify v5 and v6, but only on Android. |
Hi @BuildingFiles since I saw expo in your dependencies list, could you confirm, were seeing this issue while building with expo-go, or were you running the native app directly? And it looks like you are developing a web app using expo, correct? (the code example you pasted is Web specific.) I can produce a similar error with expo@51 using expo Web while I couldn't get a Web app using expo@48 to run. However, Amplify JS doesn't support expo Web, nor React Native Web. |
We are using expo to deploy both web and mobile versions of the app. We do use expo-go when testing the mobile app. However for the web browser version that does not apply. We have been developing and launched this app over the last several years and the web version has always run without a problem. Up till upgrading past V5.0.11. The vast majority of our clients use the web version as well. if(platform.includes('web') === true){
const file = await Storage.get(fileKey, { download: true });
const blob = await file.Body.blob();
const reader = new FileReader();
reader.readAsDataURL(blob);
reader.onloadend = async() => {
const url = reader.result;
try{
const a = document.createElement('a');
a.href = url;
a.download = fileName || 'download';
const clickHandler = () => {
setTimeout(() => {
a.removeEventListener('click', clickHandler);
}, 150);
};
a.addEventListener('click', clickHandler, false);
a.click();
}catch(e){
console.log(e);
setDialogArr({display:true, type:'alert', message: 'ERROR: failed to download file!'});
}
}
}else{
try{
const fileUrl = await Storage.get(fileKey, {expires: 600, download: false});
const fileUri = FileSystem.cacheDirectory + fileName;
const fileInfo = await FileSystem.getInfoAsync(fileUri);
if (!fileInfo.exists) {
console.log("Downloading File");
const { uri } = await FileSystem.downloadAsync(fileUrl, fileUri);
await Sharing.shareAsync(uri);
}else{
console.log("File exists!");
await Sharing.shareAsync(fileInfo.uri);
}
}catch(e){
console.log(e);
}
} As far as I can tell some change in the aws-amplify backend after 5.0.11 caused the bug. Impacting how storage.get downloads blob data. The response returned from the server is directly different then it was beforehand. In the live versions of our application response.Body does not contain the V6 architecture with .blob() .text() or .json(). It simply is a blob as expected via the documentation. If you know the section of code in the git repo that handles this response I'd be happy to take a look with you. I was trying to locate it but have not had any luck navigating the files yet. |
Thanks for the follow-up and additional details, @BuildingFiles! In the later version of Amplify JS v5, to reduce the bundle size, the library has removed usage of AWS S3 SDK and Axios. With this effort, the team also tried to align the behavior of the underlying response object as a standard HTTP client rather than the custom behavior used by the SDK. This might've introduced an accidental breaking change towards Expo Web/React Native Web, as the library doesn't have any tests against those not officially supported platforms. In the meantime, what the documentation showed is working correctly on a "standard" Web app. I think the best way to resolve this for you is to adapt the extra step |
I will test the android deployment and see if it is also getting the same responses for the get function. I don't mind using my workaround if its the final and official solution. But if that is the case then they should update the documentation for the react-native V5 storage.get section. If amplify-js does not officially support react-native and expo, then is this not the correct git repo for this bug? They are clearly official options in the amplify documentation and we have been developing our app for the last 4 years using them. So is there some place else this bug should be submitted? |
Hi @BuildingFiles Thanks for the follow up.
Yes that's a great suggestions, I've sent a PR to remove the incorrect example from the documentation.
Amplify JS does support react-native and Expo, but it doesn't support react-native Web, Expo Web and Expo Go. In addition, if you are attempting to call |
Excellent, hopefully if we have to upgrade another v5 package in the future it doesn't undo the "fix"
Unfortunately, the issue is both in our web app and in the expo-go / android app. So it doesn't appear to be related to being web only. The get command directly returns a non blob regardless of the platform as far as I can tell. However if that is the case then why does the amplify documentation ONLY show a web based example. from ~ https://docs.amplify.aws/gen1/react-native/prev/build-a-backend/storage/download/ export function downloadBlob(blob, filename) {
const url = URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = filename || 'download';
const clickHandler = () => {
setTimeout(() => {
URL.revokeObjectURL(url);
a.removeEventListener('click', clickHandler);
}, 150);
};
a.addEventListener('click', clickHandler, false);
a.click();
return a;
}
// usage
async function download() {
const result = await Storage.get(fileKey, { download: true });
downloadBlob(result.Body, 'filename');
} Android, and I'm guessing iOs do not support the use of a.href. When we implemented our version of this code years ago we had to wrap it in a platform check so it only was used in the "web" version of our app. The test I did for my last post on android removed the platform check and commented out the android specific code we use to confirm that the storage.get function was indeed broken in both web and expo go apps
We use expo go, and our app was built from the ground up. So this "prebuilt" thing you mentioned doesn't sound like it applies. Also most of our app uses reader.readAsDataURL and gets the same exact error ".gets body is not a blob". |
This is still wrong in several ways and will not work. #1 The change here doesn't apply to older versions of V5 only the newest ones. That distinction should be made somewhere #2 The code sample given is wrong and will not function on any platform. Also has incorrect comments. const result = await Storage.get(`filename.txt`, { download: true });
// data.Body is a Blob
result.Body.text().then((string) => {
// handle the String data return String
}); The comment "// data.Body is a Blob" is incorrect it is not a blob in these later versions of V5 Also "result.Body.text().then((string) => {" is missing async It should be more like it is shown in V6. But using storage.get from V5 since its all scrambled together now. try {
const downloadResult = await Storage.get({
path: 'public/album/2024/1.jpg',
// Alternatively, path: ({identityId}) => `protected/${identityId}/album/2024/1.jpg`
{ download: true },
}).result;
const text = await downloadResult.body.text();
// Alternatively, you can use `downloadResult.body.blob()`
// or `downloadResult.body.json()` get read body in Blob or JSON format.
console.log('Succeed: ', text);
} catch (error) {
console.log('Error : ', error);
} |
Hi @BuildingFiles thanks for the follow up!
As I mentioned earlier,
The code example is using the promise chaining, within the I do personally like your example better with try/catch and async/await, please feel free to open a PR towards https://github.com/aws-amplify/docs, we appreciate your contribution! |
You are correct I didn't see the
I'm going to be earnest. I don't really understand what you mean when you say react-native-web and expo-web is not supported. Creating an expo app installs react-native-web by default. "npx expo start" won't even run if it isn't installed. As far as I can tell you have to have it even if your only deploying to expo go. Where the bug also is very much present. Our app isn't using expo-web. I don't think anyone is. Its a 6 year old, unmaintained package with no dependents. So when you say "a standard Web app" what do you mean? Is their some other way to use expo / react-native with amplify in a browser? Or do you mean running expo or react-native apps in a browser isn't supported by Amplify? |
By a standard Web app I meant a Web app built with JavaScript/Typescript without react-native/expo.
Amplify JS functionalities are not guaranteed to be stable or usable when you run the react-native/expo developed app as a Web app.
Yes, that's correct.
This is what Expo prefers as a part of their development flow, where you can run your native app as a Web app to see the result. As Amplify JS doesn't support Expo Web or Expo Go, and you also mentioned "Our app isn't using expo-web", we recommend you develop your app directly with prebuilt native apps that run on Android/iOS emulator/simulator. You can generate the native apps by running
Sorry about the inconvenience, the documentation site provides a You also mentioned "with very little details on deploying to mobile platforms.", what details are you looking for in particular? Happy to help here. |
I dug a bit deeper to see what's changed. Expand to see the test script.
What has changed
What's not working The metro bundler seemed not picking up the above module resolution rule, so the metro bundler built Web app uses the The metro bundler seemed requiring explicit extension name on the left side. attempted to add the extension name, the metro bundler can now pick up the resolution. Hi @BuildingFiles could you do me a favor, open the "browser": {
"./lib-esm/AwsClients/S3/runtime/index.js": "./lib-esm/AwsClients/S3/runtime/index.browser.js"
}, Then rerun Note: this metro bundler module resolution issue doesn't exist in v6. |
Thank you very much for looking into this further. I added the I am still currently on expo 48 since we have been testing this, so I have not upgraded further yet. |
Could you please clarify the following, @BuildingFiles
|
yes But I was incorrect, it is working. The result was not what I anticipated. Sorry for the confusion. Prior to the edit, the response data for body was a "readable stream" and contained the 3 functions Afterwards Body is a "blob", BUT also still contains the 3 functions To further confirm, under the network tab in dev tools. You were correct, the current version of the package is triggering fetch.js. And after your change its showing With your edit |
Great!
I believe this is expected behavior, the intention was keeping the We will release a new patch version for v5 soon that contains the |
Excellent I believe that will fix this issue. |
Hi @BuildingFiles the fix is now available with |
@HuiSF I have the new version installed and it appears to be working. I believe this issue has been resolved so I'll close the ticket. Thank you again for all the help. |
Before opening, please confirm:
JavaScript Framework
React Native
Amplify APIs
Storage
Amplify Version
v5
Amplify Categories
storage
Backend
Amplify CLI
Environment information
Describe the bug
After upgrading AWS-Amplify from Version 5.0.11 to 5.3.23 all our file download and several in app file loading systems stopped working.
When executing
const result = await Storage.get(fileKey, { download: true });
TypeError: Failed to execute 'readAsDataURL' on 'FileReader': parameter 1 is not of type 'Blob'.
Or
TypeError: Failed to execute 'createObjectURL' on 'URL': parameter 1 is not of type 'Blob'.
depending on which file file reading method we tried.
The problem appears to be that result.Body now has 3 sub functions, .text() .json() and .blob(). Which matches what is shown in the documentation for V6 but does not for V5.
Expected behavior
Storage.get
should be returning "result.Body" as a blob, Not as a parent for 3 sub functions.Reproduction steps
Code Snippet
Code compliant with V5 documentation that reproduces the Issue.
Manual configuration
No response
Additional configuration
Mobile Device
No response
Mobile Operating System
No response
Mobile Browser
No response
Mobile Browser Version
No response
Additional information and screenshots
Temporary workaround I came up with after much testing, reviewing the V6 documentation, and applying its response processing to V5.
The text was updated successfully, but these errors were encountered: