DigitalCommons / land-explorer-back-end

GNU Affero General Public License v3.0
0 stars 0 forks source link

[SF&G] Corrupted map element breaking 'Open' maps #58

Open lin-d-hop opened 6 months ago

lin-d-hop commented 6 months ago

Description

A user is unable to open any maps.

Image

BE gives a 500 error preventing the listing of any maps.

@rogup did some detective work:

When there's a problem on the backend, I usually ssh onto the server and look at the pm2 error logs (which are for the node app) with the command pm2 logs --err. They're pretty rubbish logs since we don't use a proper logging library, but in this case they're showing the error: error getting user maps: Cannot read properties of null (reading 'name')

So we're hitting an error when getting the name property of some object that is null, and it must be the first property that we try to access, otherwise it would hit the error earlier. I've searched through the code for .name and I think this is the only example that fits the criteria, in map.ts:

const polygon = await Polygon.findOne({
      where: {
        idpolygons: mapMembership.item_id,
      },
    });
    polygonsAndLines.push({
      name: polygon.name,
      description: polygon.description,
      type: "Polygon",
      // Form GeoJSON that is used by front end
      data: {
        id: polygon.uuid,
        type: "Feature",
        properties: {},
        geometry: {
          type: "Polygon",
          coordinates: polygon.vertices.coordinates,
        },
      },
      center: polygon.center.coordinates,
      length: polygon.length,
      area: polygon.area,
      uuid: polygon.uuid,
    });

So there seems to be a lingering map_membership for a line or poly in the user's map, where the map_membership exists but the object itself has been deleted. Which is strange and shouldn't happen. I think you could find out which map_membership this is by doing some SQL queries and cross-referencing the results. Deleting the map_membership should fix the problem for this user. Then we could think about how we want to avoid the bug more generally with a code fix.

How to replicate

DM Lynne to get the log in details for the person experiencing the issue.

Acceptance Criteria

  1. Open map is working for this user
  2. A proposal on how to prevent this from happening. Implement it if it is an XS (less than 1/2 day) task.
ms0ur1s commented 5 months ago

@lin-d-hop the specified user's maps are now available.

image

Thank you @rogup for pairing with me and formulating the below query searching for map_memberships that persist after deletion, while the items (markers, polygons or lines) relating to the specified map_membership have been successfully removed.

User Specific Query

SELECT * 
FROM map_memberships 
LEFT JOIN user_map 
ON user_map.map_id = map_memberships.map_id 
WHERE user_id = #### 
AND item_type_id = 1 
AND item_id NOT IN (SELECT idpolygons FROM polygons);

Check On All Users

SELECT * 
FROM map_memberships 
LEFT JOIN user_map 
ON user_map.map_id = map_memberships.map_id 
WHERE item_type_id = 1 
AND item_id NOT IN (SELECT idpolygons FROM polygons);


While the exact cause is not clear, @rogup theorised that this scenario is due to a crash during the updateMap process in which any objects which have changed are removed from the DB and then re-added, or just removed, with the destruction of MapMembership taking place after the other objects.

By way of preventing this MapMembership.destroy has been moved above operations that remove markers, polygons etc. within this process. Code refactor implemented in PR https://github.com/DigitalCommons/land-explorer-back-end/pull/59

From queries\maps.ts

 if (markerIds.length) {
    console.log(
      `Removing ${markerIds.length} markers from DB for map ${mapId}`
    );
    await Marker.destroy({
      where: { idmarkers: markerIds },
    });
  }

  if (polygonIds.length) {
    console.log(
      `Removing ${polygonIds.length} polygons from DB for map ${mapId}`
    );
    await Polygon.destroy({
      where: { idpolygons: polygonIds },
    });
  }

  if (lineIds.length) {
    console.log(`Removing ${lineIds.length} lines from DB for map ${mapId}`);
    await Line.destroy({
      where: { idlinestrings: lineIds },
    });
  }

  \\ MapMemberships removed here

  await MapMembership.destroy({
    where: { map_id: mapId },
  });
lin-d-hop commented 5 months ago

Awesome, thanks team! I'll confirm with the user. Sounds like a good solution to prevent this from happening again :)

lin-d-hop commented 5 months ago

Confirmed all working for the user :)