Array.push works strange when called in promise.then or promise.catch

Question

I wrote a method that should return a list of devices connected to the network that the client has.

export const connectedDevicesCore = (vpnId: string, vpnAuthToken: string) =>
    Service.listVPNConnectionsCore(vpnId, vpnAuthToken).then(vpnConnectedDevices => {
        let listPromises: Promise<any>[] = [];
        let listDevices: IDeviceData[] = [];
        vpnConnectedDevices.data.forEach((member: IZeroTierMember) => {
            listPromises.push(apiUtils.ApiGet(`http://${member.config.ipAssignments[0]}:5000/api/Main`).then((response: any) => {
                let device: IDeviceData = response.data;
                device.IPVpn = member.config.ipAssignments[0];
                listDevices.push(device);
            }).catch(() => {
                let device: IDeviceData = IDeviceExample;
                device.IPVpn = member.config.ipAssignments[0];
                device.DeviceName = member.name;
                device.Status = 'Have no client';
                listDevices.push(device);
            }));
        });
        return Promise.allSettled(listPromises).then(() => listDevices);
    });

ApiGet method:

export const ApiGet = (apiString: string, authToken: string | null = null) => {
    let config = { headers: { Authorization: `Bearer ${authToken}` } };
    let promise = axios.get(apiString, authToken != null ? config : undefined);
    return promise
};

The idea was that if the client responded to the request, then the data that he transferred should be added to the array with all devices, if the requested client does not respond (i.e., is absent) Add a device with the same IP and information that he is without a client.

The problem is that writing data to the array is very strange, not the way it always is.

Example 1: There are 3 devices on the network, each without a client.

1. When the first request returns the "ETIMEDOUT" error, listDevices looks like this:

[ { DeviceName: 'HP Server',
    ID: 0,
    IP: '',
    IPVpn: '192.168.193.227',
    MAC_Adress: '',
    Status: 'Have no client',
    AudioOutputs: [] } ]

2. When the second request returns the "ETIMEDOUT" error, listDevices looks like this:

[ { DeviceName: 'Huawei P Smart 2021',
    ID: 0,
    IP: '',
    IPVpn: '192.168.193.64',
    MAC_Adress: '',
    Status: 'Have no client',
    AudioOutputs: [] },
  { DeviceName: 'Huawei P Smart 2021',
    ID: 0,
    IP: '',
    IPVpn: '192.168.193.64',
    MAC_Adress: '',
    Status: 'Have no client',
    AudioOutputs: [] } ]

3. When the third request returns the "ETIMEDOUT" error, listDevices looks like this:

[ { DeviceName: 'Test Client',
    ID: 0,
    IP: '',
    IPVpn: '192.168.193.147',
    MAC_Adress: '',
    Status: 'Have no client',
    AudioOutputs: [] },
  { DeviceName: 'Test Client',
    ID: 0,
    IP: '',
    IPVpn: '192.168.193.147',
    MAC_Adress: '',
    Status: 'Have no client',
    AudioOutputs: [] },
  { DeviceName: 'Test Client',
    ID: 0,
    IP: '',
    IPVpn: '192.168.193.147',
    MAC_Adress: '',
    Status: 'Have no client',
    AudioOutputs: [] } ]

Example 2: There are 3 devices on the network, only the first device has a client.

1. The first request writes the data returned by the client to listDevices.
2. The second request behaves as in example 1, only it does not overwrite the response of the first client.
3. The third request behaves as in example 1, only it does not overwrite the response of the first client.

That is, for example 2, the final answer will look like this:

[ { DeviceName: 'Test Client',
    ID: 0,
    IP: '192.168.8.3',
    IPVpn: '192.168.193.147',
    MAC_Adress: 'C0B883046C3F',
    Status: 'Succes',
    AudioOutputs: [...{sth: "sth"}...] },
  { DeviceName: 'Huawei P Smart 2021',
    ID: 0,
    IP: '',
    IPVpn: '192.168.193.64',
    MAC_Adress: '',
    Status: 'Have no client',
    AudioOutputs: [] },
  { DeviceName: 'Huawei P Smart 2021',
    ID: 0,
    IP: '',
    IPVpn: '192.168.193.64',
    MAC_Adress: '',
    Status: 'Have no client',
    AudioOutputs: [] } ]

I tried many variations of this code, keeping the logical idea, but alas, it did not help in any way.

This code runs on Node.js 10.23.0 + Typescript 4.2.3

Any ideas what I'm doing wrong?

Answer 1

I think the problem is that in the failure case, you're not making a copy of IDeviceExample (which is oddly-named — I'd expect that to be an interface, but you're using it like an object). In your catch handler, change:

let device: IDeviceData = IDeviceExample;

to

let device: IDeviceData = {...IDeviceExample};

so that you're creating a new object, and assigning to device.IPVpn on that object won't affect other places you're using it. (Or if a deep copy is needed, see here. The code in the question doesn't need a deep copy, but I don't know what you do with these device objects in other code.)

---

Side note: Rather than pushing to a list as the promises settle and using allSettled, I suggest handling rejection inline (as you are) and using Promise.all without the extra array (Promise.all won't see any rejections, since you're handling them inline). Also, if response.data is IDeviceData, I suggest using {data: IDeviceData} as the type for response rather than any (in general, avoid any). In fact, you don't need to specify the type explicitly if apiUtils.ApiGet has a generic type parameter. Here's an example of that (also including changing the name IDeviceExample to DeviceExample per your comment below):

export const connectedDevicesCore = (vpnId: string, vpnAuthToken: string) =>
    Service.listVPNConnectionsCore(vpnId, vpnAuthToken).then(vpnConnectedDevices => {
        const listPromises = Promise.all(vpnConnectedDevices.data.map(member => {
        //        ^ TypeScript can infer the type of this rather than your having to specify it explicitly (and no need for `any`)
            return apiUtils.ApiGet<IDeviceData>(`http://${member.config.ipAssignments[0]}:5000/api/Main`)
        //                        ^^^^^^^^^^^^^−−−− *** type argument saying what the response will be
                .then(({data: device}) => { // *** No need for `any`, and you can use destructuring to put `data` in `device`
                    device.IPVpn = member.config.ipAssignments[0];
                    return device;
                })
                .catch(() => {
                    let device: IDeviceData = {...DeviceExample}; // *** Copy the example object
                    device.IPVpn = member.config.ipAssignments[0];
                    device.DeviceName = member.name;
                    device.Status = "Have no client";
                    return device; // *** Converts rejection to fulfillment with this value
                });
        }));
        return listPromises;
    });

Just FWIW. :-) Playground link