HuiSF
commented
1 month ago Thanks for reporting this issue! We will look into this.
Closed BuildingFiles closed 1 month ago
HuiSF
commented
1 month ago Thanks for reporting this issue! We will look into this.
HuiSF
commented
1 month ago It looks like the react-native-url-polyfill couldn't look up the BlobModule from react-native NativeModule and it throws, will look into a solution for this.
Update: this issue is actually reproducible with both Amplify v5 and v6, but only on Android. Update2: the error I encountered while testing on Android is actually different from the issue that the OP posted. (I will handle this separately)
HuiSF
commented
1 month ago 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.
BuildingFiles
commented
1 month ago 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. However, it is specifically the web version running into the reported issue so far. I have not tested on mobile yet. The mobile version of the download code does not use "download: true" in storage.get, so I believe it will not have the same problem since it's not meant to download a blob.
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. That said you are correct you cant run some of the web specific code in the expo-go mobile application. You need to make a few conditional sections of code like the download function below. (this contains the blob workaround I found) which has it fully operational for now.
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.
HuiSF
commented
1 month ago 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 await result.Body.blob().
BuildingFiles
commented
1 month ago 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
await result.Body.blob().
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. Found here: https://docs.amplify.aws/gen1/react-native/prev/build-a-backend/storage/download/
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?
BuildingFiles
commented
1 month ago So after testing in an avd through expo-go I get the exact same error and response from storage.get unless I apply my fix.
The response still contains the .json .text and .blob functions instead of body simply being a blob as expected for V5.
HuiSF
commented
1 month ago Hi @BuildingFiles Thanks for the follow up.
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.
Yes that's a great suggestions, I've sent a PR to remove the incorrect example from the documentation.
If amplify-js does not officially support react-native and expo, then is this not the correct git repo for this bug?
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 URL.createObjectURL() from Android (in a prebuilt Expo Android app, not Expo Go) you may need update the AndroidManifest.xml following the requirement of react-native-url-polyfill.
BuildingFiles
commented
1 month ago Yes that's a great suggestions, I've sent a PR to remove the incorrect example from the documentation.
Excellent, hopefully if we have to upgrade another v5 package in the future it doesn't undo the "fix"
Amplify JS does support react-native and Expo, but it doesn't support react-native Web, Expo Web and Expo Go.
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
In addition, if you are attempting to call
URL.createObjectURL()from Android (in a prebuilt Expo Android app, not Expo Go) you may need update theAndroidManifest.xmlfollowing the requirement ofreact-native-url-polyfill.
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". I have been testing and using with createDataURL to avoid any confusion on your end since the documentation uses it. That said applying my "fix" resolved the blob problem for both READER and URL, in both Web and Expo go. Not to mention that the directly logged output from .get shows 3 functions for body and not a blob. So the problem lies in the response from .get and not how the response is being processed after the fact in our app.
cwomack
commented
1 month ago @BuildingFiles with docs PR #8002 merged in, this should accomplish the documentation changes requested within this issue. At this point, are there any other blockers for you after applying the workaround? Thanks!
BuildingFiles
commented
1 month ago @BuildingFiles with docs PR #8002 merged in, this should accomplish the documentation changes requested within this issue. At this point, are there any other blockers for you after applying the workaround? Thanks!
This is still wrong in several ways and will not work.
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 The .text() is a function and must be executed. It does not contain any data from the initial response. So until it's ran it will fail to download anything.
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!
The change here doesn't apply to older versions of V5 only the newest ones. That distinction should be made somewhere The comment "// data.Body is a Blob" is incorrect it is not a blob in these later versions of V5
As I mentioned earlier, response.Body is still a Blob for backwards compatibility within a standard Web app even with later versions of v5. The type would change only in Expo Web/react-native Web (unsupported) from my testing.
Also "result.Body.text().then((string) => {" is missing async
The code example is using the promise chaining, within the .then() clause you should get the text content, which is an equivalent to const text = await downloadResult.body.text().
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!
BuildingFiles
commented
1 month ago The code example is using the promise chaining, within the
.then()clause you should get the text content, which is an equivalent toconst text = await downloadResult.body.text().
You are correct I didn't see the .then(). That would work as well.
As I mentioned earlier,
response.Bodyis still aBlobfor backwards compatibility within a standard Web app even with later versions of v5. The type would change only in Expo Web/react-native Web (unsupported) from my testing.
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?
Because we started developing our application over 4 years ago on Amplify. And it has always been marketed as a "full stack", "cross platform" system. With react-native and expo as 2 of the options to do so. Not to mention the documentation is clearly focused on the web version with very little details on deploying to mobile platforms. So hearing this now is not only terrifying but extremely concerning.
HuiSF
commented
1 month ago Hi @BuildingFiles
So when you say "a standard Web app" what do you mean?
By a standard Web app I meant a Web app built with JavaScript/Typescript without react-native/expo.
what you mean when you say react-native-web and expo-web is not supported.
Amplify JS functionalities are not guaranteed to be stable or usable when you run the react-native/expo developed app as a Web app.
Or do you mean running expo or react-native apps in a browser isn't supported by Amplify?
Yes, that's correct.
Creating an expo app installs react-native-web by default.
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 npx expo prebuild. See expo documentation for more details.
Not to mention the documentation is clearly focused on the web version with very little details on deploying to mobile platforms.
Sorry about the inconvenience, the documentation site provides a react-native selector, when you select it the pages show display react-native specific contents. As the Amplify JS API interfaces are the same between Web and react-native, so most of the code examples can be used on both platforms. If you find any errors and improvement suggestions please always feel free to open an issue.
You also mentioned "with very little details on deploying to mobile platforms.", what details are you looking for in particular? Happy to help here.
HuiSF
commented
1 month ago I dug a bit deeper to see what's changed.
Hi @BuildingFiles could you do me a favor, open the package.json file located under <your_project_root>/node_modules/@aws-amplify/storage, change the module resolution entry under the "browser" field as the following (adding the .js extension):
"browser": {
"./lib-esm/AwsClients/S3/runtime/index.js": "./lib-esm/AwsClients/S3/runtime/index.browser.js"
},Then rerun npx expo start and test again. (I can resolve the error with this testing with the Expo@51 built app)
Note: this metro bundler module resolution issue doesn't exist in v6.
BuildingFiles
commented
1 month ago I dug a bit deeper to see what's changed.
Expand to see the test script.
- I couldn't build an App with Expo (v48 version info provided in the OP) with
aws-amplify@5.3.23- I recreate an App with Expo@latest and I was able to build the App with
aws-amplify@5.3.23and rannpx expo startto view the App in browser as an Web app- Observed a
Storage.get()callWhat has changed
- the code uses the
fetchAPI for making network requests, withaws-amplify@5.0.11, however, it usesXMLHttpRequest(XHR) for making network requests. Only the latter returnsresponse.Bodyas a Blob- With removing AWS SDK, this module resolution has been introduced so it uses
XHRon Web, and usesfetchother runtimesWhat's not working
The metro bundler seemed not picking up the above module resolution rule, so the metro bundler built Web app uses the
fetchAPI which caused issue.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
package.jsonfile located under<your_project_root>/node_modules/@aws-amplify/storage, change the module resolution entry under the"browser"field as the following (adding the.jsextension):"browser": { "./lib-esm/AwsClients/S3/runtime/index.js": "./lib-esm/AwsClients/S3/runtime/index.browser.js" },Then rerun
npx expo startand test again. (I can resolve the error with this testing with the Expo@51 built app)Note: this metro bundler module resolution issue doesn't exist in v6.
Thank you very much for looking into this further.
I added the .js to the browser field in node_modules/@aws-amplify/storage as requested. However, the output of storage.get() did not change after starting up the app.
I am still currently on expo 48 since we have been testing this, so I have not upgraded further yet.
HuiSF
commented
1 month ago Could you please clarify the following, @BuildingFiles
npx expo start with Expo version 48 + aws-amplify@5.3.23? package.json?BuildingFiles
commented
1 month ago Could you please clarify the following, @BuildingFiles
- Did your project build when run
npx expo startwith Expo version 48 + aws-amplify@5.3.23?- Have you cleared all bundle caches (refer to the Expo documentation) after making the change to
package.json?
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 .json() .text() .blob().
Afterwards Body is a "blob", BUT also still contains the 3 functions .json() .text() .blob(). So when I tested the change, I was expecting our download code which has my fix in it to throw an error that Body.blob() was undefined. When I saw the response bodys contents still hand the 3 functions in it. I had mistakenly believed the change didn't work. I failed to notice that readable stream now said blob.
To further confirm, under the network tab in dev tools. You were correct, the current version of the package is triggering fetch.js. @ G:\Repos\CodeCommit\BuildingFiles\node_modules\@aws-amplify\core\lib-esm\clients\handlers\fetch.js:58
And after your change its showing @ G:\Repos\CodeCommit\BuildingFiles\node_modules\@aws-amplify\storage\lib-esm\AwsClients\S3\runtime\xhrTransferHandler.js:35
With your edit storage.get(path,{download: true}) returns both the V6 and V5 response in .body. Which I'm not sure if that's a possible issue, or more of a bonus side effect.
HuiSF
commented
1 month ago Great!
With your edit storage.get(path,{download: true}) returns both the V6 and V5 response in .body. Which I'm not sure if that's a possible issue, or more of a bonus side effect.
I believe this is expected behavior, the intention was keeping the response.Body as a Blob with using XHR for backwards compatibility, the .json(), .blob() and .text() are standard methods for fetch returned responses.
We will release a new patch version for v5 soon that contains the package.json fix.
BuildingFiles
commented
1 month ago Great!
With your edit storage.get(path,{download: true}) returns both the V6 and V5 response in .body. Which I'm not sure if that's a possible issue, or more of a bonus side effect.
I believe this is expected behavior, the intention was keeping the
response.Bodyas aBlobwith usingXHRfor backwards compatibility, the.json(),.blob()and.text()are standard methods forfetchreturned responses.We will release a new patch version for v5 soon that contains the
package.jsonfix.
Excellent I believe that will fix this issue.
HuiSF
commented
1 month ago Hi @BuildingFiles the fix is now available with aws-amplify@5.3.24.
BuildingFiles
commented
1 month ago @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.getshould 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.