Rapid icon indicating copy to clipboard operation
Rapid copied to clipboard

Cannot restore local changes which failed to upload during OSM.org outage

Open camillemarie opened this issue 1 year ago • 8 comments

Description

I pressed the save button during the openstreetmap.org DDOS outage last night, and my changes were not uploaded (infinite spinning wheel). I closed my laptop after about 20 minutes, and it restarted at some point overnight.

When I reopened my browser today, the changes appear to still be intact in my local storage, but Rapid freezes after clicking "Restore my changes". No changes appear on the map, and the change stack buttons (save, undo, redo) are all disabled. The console shows errors about one of the new ways not being found.

This sounds a bit like #1126 , and might be tricky to fix completely. If you know of any workarounds that I can use to restore my changes and upload them, please let me know.

Here is the value of the Rapid_https://rapideditor.org_saved_history key in my storage: 2024_07_11_celamiri_Rapid_saved_history.json

Console errors (note that these are not from the first time I refreshed - I have tried to refresh multiple times and may have further broken the state since the original attempt):

Uncaught Error: Entity w-14 not found entity Graph.js:96 parentWays Graph.js:200 parentWays Graph.js:200 _includeParents Tree.js:122 _setCurrentGraph Tree.js:168 intersects Tree.js:225 intersects EditSystem.js:725 render PixiLayerOsm.js:136 render PixiScene.js:151 _app PixiRenderer.js:515 _tick PixiRenderer.js:278 emit TickerListener.ts:67 update Ticker.ts:427 _tick Ticker.ts:129 _tick Ticker.ts:133 _tick Ticker.ts:133 _tick Ticker.ts:133 Graph.js:96:12

Uncaught (in promise) Error: Entity w-14 not found entity Graph.js:96 parentWays Graph.js:200 parentWays Graph.js:200 complete Difference.js:215 render MapSystem.js:248 emit index.js:202 _updateChanges EditSystem.js:1344 d EditSystem.js:1157 fromJSONAsync EditSystem.js:1163 fromJSONAsync EditSystem.js:1163 restoreBackup EditSystem.js:1222 promise callbackrestoreBackup EditSystem.js:1222 a restore.js:38 QM on.js:3 ZM on.js:42 Jx each.js:5 yv on.js:65 U6 restore.js:37 Xx call.js:4 render UiSystem.js:439 startAsync UiSystem.js:147 initAsync Context.js:176 initAsync Context.js:176 promise callbackinitAsync Context.js:176 checkScript edit:65 EventHandlerNonNull* edit:33 Graph.js:96:12

Screenshots

https://github.com/user-attachments/assets/20fd3d1a-e7fc-4abd-a64b-cda4144deaf9

Version

2.3.1

What browser are you seeing the problem on? What version are you running?

Firefox 127.0.2 (64-bit)

The OS you're using

Windows 11

Steps to reproduce

  1. Upload of changeset fails (due to 5xx on openstreetmap.org side)
  2. Restart computer
  3. Open browser again
  4. Click "Restore my changes"

The browser URL at the time you encountered the bug

https://rapideditor.org/edit#map=18.31/47.61994/-122.01490&background=Bing&datasets=fbRoads,msBuildings&disable_features=boundaries

The auto-detected useragent string for your browser (leave blank if you're manually filling this form out)

No response

camillemarie avatar Jul 11 '24 22:07 camillemarie

Looking through the history file, w-14 was likely a "construction way" which I used for straightening/squaring part of a large building but deleted later.

Just for fun, I went through the changes and removed all references to w-14 and copied the new json value into the Rapid_https://rapideditor.org_saved_history key. Upon loading, the console then showed errors about w-26. Looking through the file, w-26 was also a construction way (this time a circular one to help define a curve) which I deleted later. So the bug seems to be specifically related to ways which do not exist in the resulting changeset.

camillemarie avatar Jul 12 '24 22:07 camillemarie

I was able to upload my changes by copying the locally stored changes into ID (Changeset 153874063), so this is a Rapid-specific issue.

Workaround steps:

  1. Go to rapideditor.org. Copy the contents of the Rapid_https://rapideditor.org_saved_history key from local storage
  2. Open ID (openstreetmap.org/edit) in a new tab. Paste the contents of the previously copied value into iD_https://www.openstreetmap.org_saved_history
  3. Refresh ID and select "Restore my changes". The changes should now load
  4. Check over the changes to make sure they loaded properly, then Save.

Of course, by successfully uploading the changes, I have made this issue more difficult to reproduce. Sorry!

camillemarie avatar Jul 12 '24 22:07 camillemarie

Thanks for the detailed analysis! The hint about the "construction ways" sounds like the sort of thing that our backup code might be missing..

bhousel avatar Jul 13 '24 02:07 bhousel

I'm having the same issue, and I was able to restore it with your workaround. Do note that you have to make any change, refresh, and then while the restore changes popup is present, replace the iD data with the one from RapiD.

Swarkin avatar Aug 02 '24 00:08 Swarkin

Screenshot (230) Same happen to me quite often, and yesterday night in particular, and I was coming to report that the "Download OSMChange File" option is Not working or do nothing

jjiglesiasg avatar Aug 05 '24 17:08 jjiglesiasg

Okay, I spent most of today trying to understand and reproduce the issue.

Here's the minimal reproduction case:

  1. Start rapid any ol' place where there are existing OSM features.
  2. Start drawing a new way by clicking an empty spot, then connect the way to an existing OSM Node.
  3. Click again on the same OSM Node (or hit ESC) to stop drawing the way. You just created a way with one new node, and one pre-existing node. (this is crucial!)
  4. Now, drag that OSM Node to a new location.
  5. Select the new way you just drew, and delete it (leaving the still-moved OSM node on the graph).
  6. Refresh Rapid and reload the changes.
  7. Observe the 🐛 .

Here's a vid I captured and a minimal .json of the changes that cause the issue:

https://github.com/user-attachments/assets/ae949e6c-e472-4eda-ad01-718fd81a126a

{
    "version": 3,
    "entities": [
        {
            "loc": [
                -122.0150828,
                47.6207117
            ],
            "id": "n-1",
            "v": 915
        },
        {
            "nodes": [
                "n-1",
                "n2507374428"
            ],
            "id": "w-1",
            "v": 1026
        },
        {
            "id": "n2507374428",
            "version": "1",
            "changeset": "18522075",
            "timestamp": "2013-10-24T15:56:14Z",
            "user": "STBrenden",
            "uid": "1340045",
            "loc": [
                -122.0155273,
                47.6208348
            ],
            "tags": {
                "highway": "turning_circle"
            },
            "v": 1034
        }
    ],
    "baseEntities": [
        {
            "id": "n2507374428",
            "version": "1",
            "changeset": "18522075",
            "timestamp": "2013-10-24T15:56:14Z",
            "user": "STBrenden",
            "uid": "1340045",
            "loc": [
                -122.015437,
                47.6208019
            ],
            "tags": {
                "highway": "turning_circle"
            },
            "v": 917
        },
        {
            "id": "w6385218",
            "version": "7",
            "changeset": "143931181",
            "timestamp": "2023-11-12T12:19:59Z",
            "user": "celamiri",
            "uid": "1528039",
            "tags": {
                "highway": "residential",
                "name": "243rd Avenue Northeast",
                "surface": "asphalt",
                "tiger:cfcc": "A41",
                "tiger:county": "King, WA",
                "tiger:name_base": "243rd",
                "tiger:name_direction_suffix": "NE",
                "tiger:name_type": "Ave"
            },
            "nodes": [
                "n53073508",
                "n11343283031",
                "n53130362",
                "n2507374402",
                "n2507374428"
            ],
            "v": 1024
        },
        {
            "id": "w777962927",
            "version": "1",
            "changeset": "81724096",
            "timestamp": "2020-03-03T11:04:01Z",
            "user": "kavshnu",
            "uid": "10192120",
            "tags": {
                "access": "private",
                "highway": "service",
                "service": "driveway"
            },
            "nodes": [
                "n7260477927",
                "n7260477928",
                "n7260477929",
                "n7260477930",
                "n7260477931",
                "n7260477932",
                "n2507374428"
            ],
            "v": 1025
        }
    ],
    "stack": [
        {},
        {
            "modified": [
                "n-1v915",
                "w-1v1026"
            ],
            "deleted": [
                "n-2"
            ],
            "annotation": "Started a line.",
            "selectedIDs": [
                "w-1"
            ],
            "transform": {
                "x": 28198009.437788706,
                "y": 12547482.838757662,
                "k": 13240955.24285964,
                "r": 0
            },
            "imageryUsed": [
                "Bing Maps Aerial"
            ]
        },
        {
            "modified": [
                "n-1v915",
                "w-1v1026",
                "n2507374428v1034"
            ],
            "deleted": [
                "n-2"
            ],
            "annotation": "Moved a node in a way.",
            "selectedIDs": [
                "n2507374428"
            ],
            "transform": {
                "x": 28198009.437788706,
                "y": 12547482.838757662,
                "k": 13240955.24285964,
                "r": 0
            },
            "imageryUsed": [
                "Bing Maps Aerial"
            ]
        },
        {
            "modified": [
                "n2507374428v1034"
            ],
            "deleted": [
                "n-1",
                "n-2",
                "w-1"
            ],
            "annotation": "Deleted a line.",
            "selectedIDs": [
                "w-1"
            ],
            "transform": {
                "x": 28198009.437788706,
                "y": 12547482.838757662,
                "k": 13240955.24285964,
                "r": 0
            },
            "imageryUsed": [
                "Bing Maps Aerial"
            ]
        }
    ],
    "nextIDs": {
        "changeset": -1,
        "node": -3,
        "way": -2,
        "relation": -1
    },
    "index": 3,
    "timestamp": 1723484237381
}

Bonkles avatar Aug 12 '24 21:08 Bonkles

Code analysis: It looks like we have a logic flaw in the history reassemblement code in EditSystem._finish().

This -finish() method walks through the history 'stack' of changes that were previously saved, one by one. At each step, _finish() grabs all the entities involved in that specific history step and then calls Graph.load(entities) on them.

Graph.load(entities) must in turn keep track of any changes that have occurred to parent ways and relations by calling _updateCalculated().

updateCalculated() is where the bug lies!

There are four edit stack states (see the 'stack' array in the above JSON):

  1. Null (start) state
  2. Create a way w-1 with new node n-1
  3. Modify w-1 by adding the OSM Node n2507374428 and deleting the transient draw node n-2
  4. Delete n-1 and w-1 as well.

When we get to the final step in the history, we call Graph.load(entities) and the entities are as follows: n-1, n-2, w-1, and n2507374428.

the values for n-1, n-2, and w-1 are all undefined as they have been deleted. Since w-1 didn't exist in the base graph, we then call this._updateCalculated(undefined,undefined), which results in a no-op (an immediate return with no adjustments to parent ways or relations).

This means that the way w-1 is still listed as a parent for n2507374428 despite its deletion. When we attempt to reconcile this later in the Map Systems change detector (used to see if we need to re-render any features as a result of the history change) we attempt to obtain the nonexistent parent ways (because those will need re-rendering). The way w-1 does not exist, and causes this error.

Bonkles avatar Aug 12 '24 22:08 Bonkles

@camillemarie thanks so much for the detailed bug report as it was absolutely crucial to figuring out where this issue was coming from! 🚀

The team will discuss how best to fix...

Bonkles avatar Aug 13 '24 19:08 Bonkles

I've done some testing the past few days and I think this bug is fixed now (thanks @Swarkin for providing a backup that triggers the issue!). I've also tested the specific steps above that cause the error, and also did a few restores by entering and leaving the walkthrough.

The analysis by @Bonkles and potential fix above is mostly correct: updateCalculated() is needed to make sure that the cached data about parent/child is updated correctly. Normally this function gets called on every edit, but when we restored a backup in fromJSONAsync(), the Graph.load(entities) function was doing it wrong.

bhousel avatar Jan 15 '25 19:01 bhousel