Amplify-js: AsyncStorage conflict between core and communit

[rogueturnip]

I wanted to put this out to help accelerate the conversion to the community version of asyncstorage.

It appears there is a race condition if both community version and core version are being used on iOS. It causes both to try to write to the same file and thus overwrite each other. See this github issue here:
https://github.com/react-native-community/react-native-async-storage/issues/118#issuecomment-500138053

[paul-doherty]

I second this. Recently found this as well. For me it manifested in logins being persisted to Async storage but closing the app and restarting it I am forced to login again. I eventually tracked it down and AsyncStorage.getItem was returning undefined for values that were present in the storage.

This is pretty serious if we cannot rely on Async storage

[rogueturnip]

I suspect moving to the community asyncstorage would be need to be a major release because switching this out in a minor release will break people that are still using the core asyncstorage in their app.

[jessedoyle]

@rogueturnip

I suspect moving to the community asyncstorage would be need to be a major release

I made a comment on another issue here with a proposal for backwards-compatible dependency injection.

For ease of reference, I propose something along the lines of:

import MyAsyncStorage from '@react-native-community/async-storage';

Amplify.configure({
  ...
  Dependencies: {
    AsyncStorage: MyAsyncStorage, // would default to react-native provided AsyncStorage
  },
  ...
});

[haverchuck]

@rogueturnip Disclaimer: I am not super familiar with this ongoing issue with RN AsyncStorage. If this issue you are having is specific to the Auth module with Amplify, could you try writing your own storage wrapper for Auth (which Amplify supports as outlined here) to use whichever specific version of AsyncStorage you want?

[rogueturnip]

Thanks! I will do that when I get back to upgrading the AsyncStorage. I looked at the custom storage wrapper example that used AsyncStorage. I couldn't get it to work. I wasn't sure where MemoryStorage came from to import it.

[tallpants]

We should move Amplify over to using @react-native-community/async-storage because AsyncStorage is being removed from React Native core in the next release (0.60) which should be out in a few weeks -- which means someone who react-native inits a new application and follows the instructions on Amplify's docs exactly will end up with a broken app.

Using your own storage wrapper should work, but it's not a good experience to have a broken app by default.

This would probably necessitate a major release since we'd need people to install and link @react-native-community/async-storage first to use Amplify (unless they're on Expo).

In the meantime I'd at least suggest adding this issue and the workaround to the docs because it's pretty serious to not be able to rely on async storage @haverchuck. I'd be happy to open a PR for it if you can tell me where in the docs it'd be most appropriate to put this!

[tallpants]

Also cc @powerful23 just so you're aware of this

[willdady]

Moving to @react-native-community/async-storage needs to be done however I think it would be good to be able to override the default with your own storage engine similar to what @jessedoyle is saying above. The problem is you can't use AsyncStorage from core _and_ the community one in the same app. It's one or the other. Being able to choose which storage engine could make the transition to using the community one easier, especially if you still have other dependencies in your app using AsyncStorage from core.

[mllrmat]

Is there any progress on this issue? This is a pressing issue for me as it breaks functionality and can lead to some very strange side effects.
E.g. If I use amplify which uses the core async storage and another dependency which uses the new one, those dependencies will overwrite each others changes.

[joelfsreis]

I would like to know as well if progress are being made. We upgraded RN to latest version and we use amplify-js, so any input on the issue update would be much appreciated

[taschik]

Is there any update from @rogueturnip ? Right now, aws-amplify is not usable with current RN 0.60.X as it can't persist the authentication. So either having a hotfix for the community version or the specification of a custom storage via configuration would be desirable.

[anthonyjoeseph]

Thanks! I will do that when I get back to upgrading the AsyncStorage. I looked at the custom storage wrapper example that used AsyncStorage. I couldn't get it to work. I wasn't sure where MemoryStorage came from to import it.

@rogueturnip using MyStorage instead of MemoryStorage works

[rogueturnip]

I switched back to the old asyncstorage for the time being because of time. @anthonyjoeseph would you be willing to post your solution for everyone?

[anthonyjoeseph]

https://gist.github.com/anthonyjoeseph/16ab868408bb874a742fdf77d0be3fd1

[anthonyjoeseph]

My guess is that the MyStorage implementation referenced in the amplify docs is an imperfectly modified version of the actual MemoryStorage implementation used in amplify, which is why there's that confusing static reference to MemoryStorage.syncPromise, and the unused static variable MyStorage.syncPromise.

[rogueturnip]

Thanks so much! if anyone from AWS is reading this it would make alot of sense to use @anthonyjoeseph example in your documentation and comment on the conflict with RN 0.60.

[taschik]

@rogueturnip I saw your post here: https://github.com/react-native-community/async-storage/issues/118 and was wondering what your final solution for the problem here is.

Are you using the custom storage workaround or did you downgrade to an older version of async-storage? and if so which version? Thanks so much for the support!

[rogueturnip]

I took the simple route and just downgraded to use the react native core asyncstorage.

Sent from Mail for Windows 10

From: Daniel Taschik
Sent: Thursday, August 22, 2019 11:10 AM
To: aws-amplify/amplify-js
Cc: rogueturnip; Mention
Subject: Re: [aws-amplify/amplify-js] AsyncStorage conflict between core andcommunit (#3422)

@rogueturnip I saw your post here: react-native-community/async-storage#118 and was wondering what your final solution for the problem here is.
Are you using the custom storage workaround or did you downgrade to an older version of async-storage? and if so which version? Thanks so much for the support!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[kirkryan]

This caused a bug that took days to track down - Please ensure that Amplify is updated to use the react-native-community async-storage, please!

[lenarmazitov]

I have same bug, and as a result I have troubles with my own redux-persist store. I can not clean it.

[TomJKlug]

This also caused a huge headache. Would greatly appreciate a switch the the react-native-community/async-storage

[mllrmat]

Is there any progress on this issue?

[mllrmat]

For all that having an issue with AsyncStorage and Amplify here is my workaround:

1. Implement your own storage class, that uses the community async storage:

import AsyncStorage from '@react-native-community/async-storage';

const MEMORY_KEY_PREFIX = '@MemoryStorage:';
let dataMemory = {};

export default class AmplifyAuthStorage {
  private static syncPromise = null;

  /**
   * This is used to set a specific item in storage
   */
  static setItem(key, value) {
    AsyncStorage.setItem(MEMORY_KEY_PREFIX + key, value, error => {
      console.log('error setting item', error);
    });
    dataMemory[key] = value;
    return dataMemory[key];
  }

  /**
   * This is used to get a specific key from storage
   */
  static getItem(key) {
    return Object.prototype.hasOwnProperty.call(dataMemory, key)
      ? dataMemory[key]
      : undefined;
  }

  /**
   * This is used to remove an item from storage
   */
  static removeItem(key) {
    AsyncStorage.removeItem(MEMORY_KEY_PREFIX + key);
    return delete dataMemory[key];
  }

  /**
   * This is used to clear the storage
   */
  static clear() {
    dataMemory = {};
    return dataMemory;
  }

  /**
   * Will sync the MemoryStorage data from AsyncStorage to storageWindow MemoryStorage
   */
  static sync() {
    if (!this.syncPromise) {
      console.log('****** sync items to memory storage!');
      this.syncPromise = new Promise((res, rej) => {
        AsyncStorage.getAllKeys((errKeys, keys) => {
          if (errKeys) {
            rej(errKeys);
          }
          const memoryKeys = keys.filter(key =>
            key.startsWith(MEMORY_KEY_PREFIX)
          );
          AsyncStorage.multiGet(memoryKeys, (err, stores) => {
            if (err) {
              rej(err);
            }
            stores.map((result, index, store) => {
              const key = store[index][0];
              const value = store[index][1];
              const memoryKey = key.replace(MEMORY_KEY_PREFIX, '');
              dataMemory[memoryKey] = value;
            });
            res();
          });
        });
      });
    }
    return this.syncPromise;
  }
}

1. Then use your storage class in the Auth configuration

Auth.configure({storage: AmplifyAuthStorage});

or

Auth: {
        identityPoolId: xxx,
        region: xxx,
        userPoolId: xxx,
        userPoolWebClientId: xxx,
        mandatorySignIn: false,
        storage: AmplifyAuthStorage
      }

1. Replace all AsyncStorage imports in your code with the community version.

Please note:

• Unfortunately the example in the documentation is very confusing (see https://github.com/aws-amplify/docs/issues/983) as it uses a non defined variable. (static member can be accessed via class name)
• You have to use the prefix @MemoryStorage: instead of @MyStorage: otherwise all your users are logged out!

It would be very nice, if help would come directly from the amplify team, on such critical issues in a commercial product.

[lvlrSajjad]

@mllrmat just replace

  private static syncPromise = null;

with

  static syncPromise = null;

[lafiosca]

One thing I don't understand just looking at this file: is there some logic elsewhere that clears syncPromise, or can the sync method effectively only ever be used once? (And if the latter, is that intentional? Just meant as an initialization method?)

Edited to answer my own question: it appears the sync method is called during the configure method: https://github.com/aws-amplify/amplify-js/blob/master/packages/auth/src/Auth.ts#L172-L175

[lafiosca]

Based on @mllrmat 's code above, I made a more TypeScript-y version: https://gist.github.com/lafiosca/774fa014d3326fcc992b55198ac5ea1d

While working on this, I found several things about the storage module interface odd; for example, setItem returns the value being set, but it doesn't appear that the current lib code uses the return value. Seems to me it would be nicer if these methods could return Promises, but that would obviously require more invasive changes to the packages.

[stale[bot]]

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.

[joebernard]

Bad bot, not stale.

[camin-mccluskey]

Any update on this? This is causing a major bug (logged out users on each app start) on a production application

[stale[bot]]

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.

[jessedoyle]

This is still very much a significant issue. Please don't mark the issue as stale.

[stale[bot]]

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.

[Off2Race]

This is still an issue. Many of us have worked around the problem but it should be fixed. Please don't mark the issue as stale.

[alexpanov]

This makes me want to migrate away from amplify. What a damn shame.

[Ashish-Nanda]

@alexpanov @Off2Race @jessedoyle

What exactly are the major issues you are facing? Is it the one mentioned in the original issue description: react-native-community/async-storage#118

Can you explain how the issue manifests in your app?

Currently AsycnStorage is still a part of ReactNative, although there is a deprecation warning it still works fine.

I had previously created a PR to migrate to the community edition of AsyncStorage #4402

However we didnt end up releasing that as expo users would need to eject their app, since the community edition of AsyncStorage was not available for expo managed apps at the time. This would be a major issue if all our expo users now had to eject their apps.

So we are currently waiting for expo to add this dependency so that we can migrate Amplify JS to use react-native-community/async-storage. However let us know your issues and maybe we can come up with a workaround.

[mauerbac]

hi @alexpanov - sorry to hear that. Following up on this -- are you still facing this issue?

[stale[bot]]

This issue has been automatically closed because of inactivity. Please open a new issue if are still encountering problems.

[iamstiil]

FYI: Expo moved to the comminity package :)
Reference