facebook / Rapid

The OpenStreetMap editor driven by open data, AI, and supercharged features
https://rapideditor.org
ISC License
509 stars 92 forks source link

Issue with partial JSON/XML responses #501

Open Bonkles opened 2 years ago

Bonkles commented 2 years ago

Description

This is a dupe of https://github.com/openstreetmap/iD/issues/6454, where we saw 'error' elements being inserted into the XML / JSON requests to the osm api.

Version

2.0.0-beta

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

all

Steps to reproduce

Nearly impossible- this bug relies on the OSM API servers to behave a certain way.

More reading: https://github.com/zerebubuth/openstreetmap-cgimap/issues/276

The browser URL at the time you encountered the bug

n/a, could happen anywhere.

Bonkles commented 2 years ago

Fixed in commit 95175530592da463413a756b90a0a076e7cdad64.

Bonkles commented 2 years ago

Also cherry-picked the fix into main branch, so v1 and v2 should have fixed this problem.

mmd-osm commented 2 years ago

Did you use catch an actual error message while the error was still present? My expectation would be to see a message like

{"error": "something went wrong loading postgres"},

rather than

{"type":"error", "message":"something went wrong loading postgres"},

I will need to double check the output, and also update the Wiki page with a JSON example.

mmd-osm commented 2 years ago

Ok, so here's a real example from the buggy 0.8.7 release:

{
  "version": "0.6",
  "generator": "CGImap 0.8.7 (22730 ubuntu)",
  "copyright": "OpenStreetMap and contributors",
  "attribution": "http://www.openstreetmap.org/copyright",
  "license": "http://opendatacommons.org/licenses/odbl/1-0/",
  "elements": [
    {
      "type": "way",
      "id": 4000392204,
      "timestamp": "2022-07-26T20:56:27Z",
      "version": 1,
      "changeset": 1874689,
      "user": "mmd2",
      "uid": 1,
      "nodes": [
        5004686582,
        5004686236,
        5004686540,
        5004686705,
        5004686546,
        5004686468,
        5004686472
      ],
      "tags": {
        "bicycle": "use_sidepath",
        "highway": "secondary",
        "maxspeed": "50",
        "name": "Bunderstraat",
        "old_ref": "N586",
        "surface": "asphalt"
      }
    },
    {
      "error": "Mismatch in tags key and value size"
    }
  ]
}
bhousel commented 2 years ago

Thank you @mmd-osm ! Can you give us an example XML also? We can incorporate these into the test suite.

mmd-osm commented 2 years ago

Sure, np, I will put those on the OSM API 0.6 wiki page as well...

<?xml version="1.0" encoding="UTF-8"?>
<osm version="0.6" generator="CGImap 0.8.7 (26234 ubuntu)" copyright="OpenStreetMap and contributors" attribution="http://www.openstreetmap.org/copyright" license="http://opendatacommons.org/licenses/odbl/1-0/">
 <way id="4000392204" visible="true" version="1" changeset="1874689" timestamp="2022-07-26T20:56:27Z" user="mmd2" uid="1">
  <nd ref="5004686582"/>
  <nd ref="5004686236"/>
  <nd ref="5004686540"/>
  <nd ref="5004686705"/>
  <nd ref="5004686546"/>
  <nd ref="5004686468"/>
  <nd ref="5004686472"/>
  <tag k="bicycle" v="use_sidepath"/>
  <tag k="highway" v="secondary"/>
  <tag k="maxspeed" v="50"/>
  <tag k="name" v="Bunderstraat"/>
  <tag k="old_ref" v="N586"/>
  <tag k="surface" v="asphalt"/>
 </way>
 <error>Mismatch in tags key and value size</error>
</osm>
mmd-osm commented 2 years ago

Pinging @Bonkles since you’ve committed the error handling code.

Not sure if you’ve noticed the updated API error message samples above. I believe the current code still needs some minor tweaks to handle API error messages properly…

Bonkles commented 2 years ago

Ooh, thanks for the ping, @mmd-osm. I'll re-open this and make the necessary changes when I'm back in the office next week.

Bonkles commented 1 year ago

Finally got around to checking this one out! @mmd-osm I do see the issue with the json parsing, and will try to get this one fixed. Tests are a bit broken now as we've taken a flamethrower to quite a lot of code since this was filed.

We'll need to get the tests back to normal and then I can fix this properly.