CODEWITHSUNDEEP
javascriptreactjspromise
ste
ste

Reputation: 3269

How to propagate resolve() in nested promises?

I'm writing a React application and in particular cases I have to resolve nested promises. The code works fine but I can't propagate the resolve() function up to the outer level, thus I'm not able to get the returning value.

This is the code:

  writeData(data) {
    this.store.dispatch({type: "START_LOADER"})

    return new Promise((resolve, reject) => {
      this.manager.isDeviceConnected(this.deviceId).then(res => {
        this.manager.startDeviceScan(null, null, (error, device) => {
          if (device.id === this.deviceId) {

            resolve("test") // -> this is propagate correctly

            device.connect().then((device) => {
              this.store.dispatch({type: "SET_STATUS", payload: "Device is connected!\n"})
              return device.discoverAllServicesAndCharacteristics()
            }).then((device) => {
              device.writeCharacteristicWithoutResponseForService(
                data.serviceId,
                data.charId,
                data.dataToWrite
              ).then(res => {

                resolve("test2") // -> this is not propagated

              }).catch(error => {
                reject(error.message)
              })
            }).catch((error) => {
              reject(error.message)
            });
          }
        });
      }).catch(error => {
        reject(error.message)
      })
    })
  }
...
...

  async writeAsyncData(data) {
    await this.writeData(data)
  }

When I call this function: 

      this.writeAsyncData({data}).then(response => {
        // here I expect whatever parameter I have passed to resolve()
        console.log(response)
      })

In case I leave resolve("test") uncommented I can console.log it without any problem, but if I comment it, the resolve("test2") doesn't show in console.log and response is undefined.

How can I make sure that even the nested parameter of the inner resolve reach the console.log ?

Upvotes: 0

Views: 888

Answers (1)

jfriend00
jfriend00

Reputation: 707806

To nest promises properly, you do NOT wrap them in yet another manually created promise. That is an anti-pattern. Instead, you return the inner promises and that will then chain them. Whatever the inner-most promise returns will be the resolved value for the whole chain.

In addition, when you have any asynchronous operations that return callbacks, you must promisify them so that you are doing all your asynchronous control flow with promises and can consistently do proper error handling also. Do not mix plain callbacks with promises. The control flow and, in particular, proper error handling gets very, very difficult. One you start with promises, make all async operations use promises.

While this code is probably simplest with async/await, I'll first show you how you properly chain all your nested promises by returning every single inner promise.

And, to simplify your nested code, it can be flattened so that rather than each level of promise making deeper indentation, you can just return the promise back to the top level and continue processing there.

To summarize these recommendations:

1. Don't wrap existing promises in another manually created promise. That's a promise anti-pattern. Besides being unnecessary, it's very easy to make mistakes with proper error handling and error propagation.

2. Promisify any plain callbacks. This lets you do all your control flow with promises which makes it a lot easier to avoid errors or tricky situations where you don't know how to propagate errors properly.

3. Return all inner promises from within the .then() handlers to properly chain them together. This allows the inner-most return value to be the resolved value of the whole promise chain. It also allows errors to properly propagate all the way up the chain.

4. Flatten the chain. If you have multiple promises chained together, flatten them so you are always returning back to the top level and not creating deeper and deeper nesting. One case where you do have to make things deeper is if you have conditionals in your promise chain (which you do not have here).

Here's your code with those recommendations applied:

// Note: I added a timeout here so it will reject
// if this.deviceId is never found
// to avoid a situation where this promise would never resolve or reject
// This would be better if startDeviceScan() could communicate back when
// it is done with the scan
findDevice(timeout = 5000) {
    return new Promise((resolve, reject) => { 
        const timer = setTimeout(() => {
             reject(new Error("findDevice hit timeout before finding match device.id"));
        }, timeout);
        this.manager.startDeviceScan(null, null, (error, device) => { 
            if (error) {
                reject(error); 
                clearTimeout(timer);
                return
            }
            if (device.id === this.deviceId) {
                resolve(device); 
                clearTimeout(timer);
            }
        });
    });
}

writeData(data) {
    this.store.dispatch({type: "START_LOADER"});
    return this.manager.isDeviceConnected(this.deviceId).then(res => {
        return this.findDevice();
    }).then(device => {
        return device.connect();
    }).then(device => {
        this.store.dispatch({type: "SET_STATUS", payload: "Device is connected!\n"})
        return device.discoverAllServicesAndCharacteristics();
    }).then(device => {
        return device.writeCharacteristicWithoutResponseForService(
            data.serviceId,
            data.charId,
            data.dataToWrite
        );
    }).then(res => {
        return "test2";  // this will be propagated
    });
}

Here's a version using async/await:

findDevice(timeout = 5000) {
    return new Promise((resolve, reject) => { 
        const timer = setTimeout(() => {
             reject(new Error("findDevice hit timeout before finding match device.id"));
        }, timeout);
        this.manager.startDeviceScan(null, null, (error, device) => { 
            if (error) {
                reject(error); 
                clearTimeout(timer);
                return
            }
            if (device.id === this.deviceId) {
                resolve(device); 
                clearTimeout(timer);
            }
        });
    });
}

async writeData(data) {        
    this.store.dispatch({type: "START_LOADER"});
    let res = await this.manager.isDeviceConnected(this.deviceId);
    let deviceA = await this.findDevice();
    let device = await deviceA.connect();
    this.store.dispatch({type: "SET_STATUS", payload: "Device is connected!\n"})
    await device.discoverAllServicesAndCharacteristics();
    let res = await device.writeCharacteristicWithoutResponseForService(
          data.serviceId,
          data.charId,
          data.dataToWrite
    );
    return "something";    // final resolved value 
}

Note: In your original code, you have two overriding definitions for device. I left that there in the first version of the code here, but changed the first one to deviceA in the second one.

Note: As your code was written, if this.manager.startDeviceScan() never finds a matching device where device.id === this.deviceId, your code will get stuck, never resolving or rejecting. This seems like a hard to find bug waiting to happen. In the absolute worst case, it should have a timeout that would reject if never found, but probably the implementation of startDeviceScan needs to communicate back when the scan is done so the outer code can reject if no matching device found.

Note: I see that you never use the resolved value from this.manager.isDeviceConnected(this.deviceId);. Why is that? Does it reject if the device is not connected. If not, this seems like a no-op (doesn't do anything useful).

Note: You call and wait for device.discoverAllServicesAndCharacteristics();, but you never use any result from it. Why is that?

Upvotes: 5

Related Questions