Diff upload 412 error message difference

[simonpoole]

The messages returned on a 412 error don't start with Precondition failed:

Example from a rails-port without cgi-map:

Precondition failed: Node 23 is still used by ways 9.

Example from the dev sandbox:

Node 4318231207 is still used by ways 4305438501.

[mmd-osm]

Do you see the same issue on prod?

[simonpoole]

Yes.

[mmd-osm]

Thanks for reporting! I guess this is just another case of regex error message parsing considered harmful. Anyway, let's take a quick look:

Relevant code in rails port: https://github.com/openstreetmap/openstreetmap-website/blob/e7ab3de654be6d0d1877eaf031baffa66bc1ed24/lib/osm.rb#L49-L61

cgimap:

• openstreetmap-cgimap/src/http.cpp

  Line 97 in a2df79d

  precondition_failed::precondition_failed(const string &message)

• openstreetmap-cgimap/src/http.cpp

  Line 78 in a2df79d

  const char *exception::what() const noexcept { return message_.c_str(); }

[mmd-osm]

One issue I see is that the Rails port doesn't adhere to the API 0.6 documentation on https://wiki.openstreetmap.org/wiki/API_v0.6. I cannot find any reference where it says that the error message should be prefixed by "Precondition Failed:".

Precondition Failed is implicitly clear by the HTTP 412 error code, i.e. there's no real need to add that text to the error message again.

@tomhughes: what would be your assessment here? Which one is right? The Rails port, the OSM API 0.6 docs, neither one?

JOSM doesn't have that issue, because they don't include "Precondition Failed:" in their regex parsing string.

[tomhughes]

The code. Always the code. The "documentation" in the wiki is written by heaven knows who based on who knows what. It's certainly not maintained by any of the developers, nor is it a specification or in any way official.

[simonpoole]

I think you are being a tad bit optimistic about the documentation, most of it was written based on the behaviour of osm.org, not as a spec (if it was, cgi-map would still be wrong as it always pluralizes ways and relations in the message(, Pinging @gravitystorm

[mmd-osm]

@simonpoole : well, the documentation says: "Note when returned as a result of a OsmChange upload operation the error messages contain a spurious plural "s" as in "... still used by ways ...", "... still used by relations ..." even when only 1 way or relation id is returned, as this implies multiple ids can be returned if the deleted object was/is a member of multiple üarent objects, these ids are seperated by commas."

This section was added by a wiki user identifying themselves as "SimonPoole" on 19. Aug. 2016‎. :)

It's really not a big deal to change this, we just need some common understanding what changes need to be done where (code + documentation).

And then there's the question of timeline. 0.8.0-dev is currently in testing, and that's where that change would be added to in the first place. It would probably stay for a bit more in dev only.
I'm not sure if this issue is urgent enough to justify a 0.7.6 release for production. Work around would be available for consumers by adjusting their regex parsing. Any comments here?

[simonpoole]

I think that proves my point that it is simply documentation of current behaviour, not a spec.

[mmd-osm]

Ok, based on MarcusWolschon/osmeditor4android@2f8c701, I'd say, we should be good to include this fix in 0.8.0, and skip the 0.7.6 release. Once the pull request gets merged into master, testing on dev will be possible. 0.8.0 release to production is still TBD.

[simonpoole]

Due to the way this fails irl (and that it isn't straightforward to actually create such a conflict without cheating) are you sure that id and JOSM don't have the error?

[mmd-osm]

Here’s the respective parser code for Josm: https://github.com/openstreetmap/josm/blob/21826a3abc29e710580a42dd0078febd0ec16d48/src/org/openstreetmap/josm/tools/ExceptionUtil.java#L105

iD sends an additional attribute to suppress the error altogether (deleting a still referenced object that is)

[mmd-osm]

Related code on Rails port: https://github.com/openstreetmap/openstreetmap-website/search?q=%22Precondition+failed%3A%22&unscoped_q=%22Precondition+failed%3A%22