Amplify-js: Storage.get('file') should throw an error if file does not exist

Created on 29 Jun 2018  路  12Comments  路  Source: aws-amplify/amplify-js

Do you want to request a feature or report a bug?
Feature

What is the current behavior?

Storage.get('file') resolves to a presigned URL, whether or not the file exists.

This is a problem because the URL results in a 404, which means an extra step is needed to check the validity of the URL.

It's especially a problem in Angular, when using constructs such as the following in the ionic aws starter project.

account.ts, lines 42-45

  refreshAvatar() {
    Storage.get(this.userId + '/avatar')
      .then(url => this.avatarPhoto = (url as string));
  }

account.html, line 12
<div *ngIf="avatarPhoto" class="avatar" [style.background-image]="'url('+ avatarPhoto +')'">

What is the expected behavior?

Storage.get('file') should return an error that can be handled using catch or if (error).

The current documentation doesn't specify what is returned on failure, so this change won't be a problem there.

However, this would be a breaking change for code that relies on the current behaviour. One option is to simply accept the breaking change. Another option is to add a third parameter to the method, for the sake of discussion, called "safe," to request this new behaviour.

Which versions of Amplify, and which browser / OS are affected by this issue? Did this work in previous versions?

0.4.6

Storage feature-request

Most helpful comment

I'm currently working with someone who experiencing this issue as well. The problem is that they can't call download because the files are very large and that would be a terrible user experience, and doing Storage.list() seems like a bad workaround as well. As file number increases it could require looking through 100s or 1000s of items to see if it exists. Is there really no better way to tell if the resource actually exists using Storage.get?

All 12 comments

Related to #1135

Dear,
Any update on this? Does this fix?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

I'm currently working with someone who experiencing this issue as well. The problem is that they can't call download because the files are very large and that would be a terrible user experience, and doing Storage.list() seems like a bad workaround as well. As file number increases it could require looking through 100s or 1000s of items to see if it exists. Is there really no better way to tell if the resource actually exists using Storage.get?

Return null or throw, but not url. Yes technically returning a 404 url is correct, but it does not make sense. The api should be practical and right now it is not.

It looks like the problem is that Storage.get isn't getting an object, but getting a signed URL to the (potentially missing) object:

https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#getSignedUrlPromise-property

Validating the existence of an object via https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#headObject-property prior to returning the signed URL could work.

That would result in 2x the API calls for even valid images, so there may be a need to introduce a new method or option if that's a problem.

However, in my own usage, getting a valid-looking URL for an object that doesn't exist resulted in several broken images.

Any update for this issue? Currently I'm just using fetch() method from React Native for accessing the presigned URL and make sure that response is 404 if file doesn't exist.

If anyone is using React Native, there is a work-around by checking the image's size.

Image.getSize(imgUrl, (width, height) => {
    console.log('image exists.')
}, err => {
    console.log('image not found')
})

If anyone is using React Native, there is a work-around by checking the image's size.

Image.getSize(imgUrl, (width, height) => {
    console.log('image exists.')
}, err => {
    console.log('image not found')
})

That'll do it! Thank you @minsanity6 This should be "fixed" though. I was definitely surprised that I didn't get an error when the key wasn't found.

+1

This feature would be useful

+1 ... there's a direct financial and performance cost to using Storage.list before Storage.get

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cosmosof picture cosmosof  路  3Comments

shinnapatthesix picture shinnapatthesix  路  3Comments

rygo6 picture rygo6  路  3Comments

epicfaace picture epicfaace  路  3Comments

leantide picture leantide  路  3Comments