sunnylqm / react-native-storage

local storage wrapper for both react-native and browser. Support size controlling, auto expiring, remote data auto syncing and getting batch data in one query.
MIT License
3.02k stars 268 forks source link

Unhandled Promise Rejection when storage.load with syncInBackground is true #214

Closed fozhao closed 5 years ago

fozhao commented 5 years ago

"react": "16.4.1", "react-native": "0.56.0", "react-native-storage": "1.0.0-beta.1", Device: iOS Simulator & real iPhone 7P.

I am calling the storage.load like this:

    const func = () => {
        storage.load({ key: DEVICE_DATA, id: device.get('dudid'), syncParams: { dmodel: device.get('dmodel') } })
            .then(payload => {
                if (typeof (payload.latest) !== 'undefined') {
                    return true
                }
                return false
            })
            .catch(err => console.log("Get data from server error: ", err))
    }

Below is the sync method:

export default function (storage) {
  const sync = params => {
    let { id, syncParams } = params

    return new Promise((resolve, reject) => {
      if (!id) {
        reject(new Error('uniqueIdentifier is undefined'))
      } else {
        return Requester.fetch(`test`, { select: 'all' })
          .then(data => {
            resolve(data)
          })
          .catch(error => {
            // When network is turned off, the fetch will fail and be caught in this catch block to reject the promise. 
            console.log("Error is caught in sync method with error: ", error)
            reject(error)
          })
      })
  }

  return sync
}

The promise's resolve works fine. But when the promise is rejected(e.g Network Error), I got a warning from React Native as shown below. In the meantime, the catch block below storage.load isn't called.

Possible Unhandled Promise Rejection (id: 1): Error: Network Error

Expected output:

Error is caught in sync method: Network Error.
Get data from server error: Network Error.

Actual output:

Error is caught in sync method: Network Error.
Possible Unhandled Promise Rejection (id: 1): Error: Network Error

When I set syncInBackground to false in storage.load call, the warning is gone and the catch block under storage.load is called.

sunnylqm commented 5 years ago

Good point. However the expected output seems impossible, since there can be only one result to return, either the cached data or the sync promise. How about passing syncInBackground to sync methods to let you decide what to do? Or do you have any suggestion?

fozhao commented 5 years ago

Hey, thanks for the quick response.

My expectation is that when the error is caught in the sync method and the promise is rejected. The catch block below storage.load can also catch that error. Now the result seems to me that when syncInBackground is set to true. The rejection of the sync promise isn't handled by the storage module.

Passing syncInBackground to sync methods might be an idea. But it will make the sync method which uses promise based result lose the ability to reject, am I right?

I might be wrong but this seems like a bug in the load method to me, probably the load method only handles the rejection of sync promise when syncInBackground is false? I tried to read the source code but honestly I haven't figured out its inner logic yet.

sunnylqm commented 5 years ago

@fozhao https://github.com/sunnylqm/react-native-storage/blob/master/src/storage.js#L206-L208 It can only return one result.

fozhao commented 5 years ago

Oh, I think I get your point. Do you mean that when syncInBackground is true, the load operation will return the cached value (if exists) instead of whatever returns from the sync method? In this case, a promise-based return value from sync method will generate Unhandled promise rejection. I don't have a solution yet but I think to have a consistent type of return value in sync method is a nice thing.

fozhao commented 5 years ago

Would about this? It will at least catch the rejection to eliminate the Unhandled error when a cached data is returned for syncInBackground load.

        if (syncInBackground) {
          this.sync[key]({ syncParams });
          .then(()=>{}).catch(err => {})
          return ret.rawData;
        }
sunnylqm commented 5 years ago

Fixed in https://github.com/sunnylqm/react-native-storage/commit/9e3dcaf86dffaa9f6308b23d377889f4d9930826