Ignore error when inserting device to database

[afathonih]

Even though there's already a guard to avoid inserting same device into database, the plugin still sees error when adding devices from nodes.

[HTTP] --> POST /device-farm/api/register?type=add
[HTTP] [{"adbPort":5037,"systemPort":57393,"sdk":"13","realDevice":true,"name":"abcdefg","busy":false,"state":"device","udid":"19191FDEExxxx","platform":"android","deviceType":"real","host":"http://a.b.c.d:4724","totalUtilizationTimeMilliSec":0,"sessionStartTime":0,"userBlocked":false,"offline":false,"meta":{"revision":0,"created":1700177328622,"version":0},"$loki":1},{"adbPort":5037,"systemPort":57404,"sdk":"11","realDevice":true,"name":"xyzab","busy":false,"state":"device","udid":"354961114044493","platform":"android","deviceType":"real","host":"http://a.b.c.d:4724","totalUtilizationTimeMilliSec":0,"sessionStartTime":0,"userBlocked":false,"offline":false,"meta":{"revision":0,"created":1700177328622,"version":0},"$loki":2},{"adbPort":5037,"systemPort":57414,"sdk":"12","realDevice":true,"name":"abcd","busy":false,"state":"device","udid":"RF8N40xxxx","platform":"android","deviceType":"real","host":"http://a.b.c.d:4724","totalUtilizationTimeMilliSec":4943261,"sessionStartTime":0,"userBloc...
[HTTP] <-- POST /device-farm/api/register?type=add 500 3 ms - 2237
[HTTP] 
Error: Document is already in collection, please use update()
    at Collection.add (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/lokijs/src/lokijs.js:6148:15)
    at Collection.insertOne (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/lokijs/src/lokijs.js:5939:17)
    at Collection.insert (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/lokijs/src/lokijs.js:5855:21)
    at /Users/ci/.appium/node_modules/appium-device-farm/lib/data-service/device-service.js:33:30
    at Array.forEach (<anonymous>)
    at addNewDevice (/Users/ci/.appium/node_modules/appium-device-farm/lib/data-service/device-service.js:30:13)
    at /Users/ci/.appium/node_modules/appium-device-farm/lib/app.js:99:43
    at Layer.handle [as handle_request] (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/express/lib/router/route.js:144:13)
    at Route.dispatch (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/express/lib/router/route.js:114:3)
    at Layer.handle [as handle_request] (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/express/lib/router/layer.js:95:5)
    at /Users/ci/.appium/node_modules/appium-device-farm/node_modules/express/lib/router/index.js:284:15
    at Function.process_params (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/express/lib/router/index.js:346:12)
    at next (/Users/ci/.appium/node_modules/appium-device-farm/node_modules/express/lib/router/index.js:280:10)
    at /Users/ci/.appium/node_modules/appium-device-farm/lib/app.js:55:12
    at Generator.next (<anonymous>)
    at fulfilled (/Users/ci/.appium/node_modules/appium-device-farm/lib/app.js:5:58)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

This PR does not fix the root cause. It adds graceful error handling and allows the rest of the devices to be added.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
appium-device-farm	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 17, 2023 8:48am

[saikrishna321]

@afathonih Can you please check why the if condition is getting satisfied?

Ideally the error is valid, we only need to make sure the filter condition is correct

[saikrishna321]

export function addNewDevice(devices: Array<IDevice>) {
  /**
   * If the newly identified devices are not in the database, then add them to the database
   */
  const devicesInDB = DeviceModel.chain().find().data();
  devices.forEach(function (device) {
    const isDeviceAlreadyPresent = devicesInDB.find(
      (d: IDevice) => d.udid === device.udid && device.host === d.host
    );
    if (!isDeviceAlreadyPresent) {
      DeviceModel.insert({
        ...device,
        offline: false,
        userBlocked: false,
      });
    }
  });
}

This function is working correct, just wrote a test. Can you check why it's not working from your remote execution.

[afathonih]

@saikrishna321 Yes. I'm going to spend a bit more time on this. Looking at the data sent by nodes, there are some loki-related fields included as well.

[afathonih]

There are two issues in the existing implementation:

1. const devicesInDB = DeviceModel.chain().find().data(); -> This part will snapshot the data and will not see any new data added in between request from nodes.
2. if (!isDeviceAlreadyPresent) { -> isDeviceAlreadyPresent is a ResultSet so it's always true-ish

The fix:

1. Always query the db on the spot before each insert.
2. Check the length of the result query to be zero.

[afathonih]

But, apparently this is the root cause:
The node send whole list of local device which also includes specific fields from lokijs such as $loki and meta.
When we insert this data into database, lokijs will use $loki field to decide where to save it. It some cases, the value is already used in the database.

Example:
Existing DB:

[
  {
     name: "foo",
     meta: {}
     $loki: "1"
  }
]

new device:

{
  name: "bar",
  $loki: "1",
  meta: {}
}

When we search with find({name: "bar"}) it will return empty result. But when we try to insert the above data, it will fail because document already exists. The $loki value clashes.