search

openstreetmap / operations

OSMF Operations Working Group issue tracking
https://operations.osmfoundation.org/
99 stars 12 forks source link

Bad Request (400) - Version may not be negative at line 1 #303

Closed westnordost closed 5 years ago

westnordost commented 5 years ago

This may as well be a bug in StreetComplete or osmapi, I am currently investigating. Maybe you immediately know what is wrong or can help me find the bug.

Since this morning, I am getting error reports from users of StreetComplete about failing upload. The error returned by the OSM API Bad Request (400) - Version may not be negative.

The newest version of the app is out for some weeks already, but I am only getting error reports about this now. So, something must have changed very recently that leads to the app making a bad request.

I can't find where in the OSM API a string like "Version may not be negative" is returned. Could you point me at it? Also, the version mentioned in that error message is the version of the element uploaded, right?

westnordost commented 5 years ago

Nevermind, I found the bug. The error message indeed means the version of the uploaded element. StreetComplete sets the version to -1 if no version attribute is sent when querying the elements from Overpass.

tomhughes commented 5 years ago

If you're using diff upload then those are now going to cgimap which may be generating different errors though I would hope the rails version would also reject a bogus version.

westnordost commented 5 years ago

I'm using POST changeset/$id/upload with the body being osmChange, that's diff upload right?

Was this changed today or in the last few days, the move to cgimap? That would explain why I get those reports now.

If that is the case, I have a hunch why this worked before at all: Maybe the previous rails implementation returned simply a 409 Conflict if the version given in the diff is not the same as the current server version? Cause that case is covered within the app - the app handles a conflict exception by re-downloading the newest version from the OSM Api and then trying again to upload the element with the changes applied.

westnordost commented 5 years ago

Reopened because if my assumption is correct, perhaps it should be considered to return a conflict error in that case so that the cgimap implementation behaves the same as the rails implementation here.

If you think, no, the new behavior is better/should stay, feel free to close this again.

tomhughes commented 5 years ago

Yes the switch happened yesterday and yes that is the call that is affected.

@mmd-osm any thoughts?

tomhughes commented 5 years ago

I'll move this ticket anyway because if it's not the rails code then this is the wrong place for it...

mmd-osm commented 5 years ago

Could you please create an issue on the cgimap repo with exact step by step description on how to reproduce the issue? Thanks!

tomhughes commented 5 years ago

I would have moved this there if I could ;-) That's https://github.com/zerebubuth/openstreetmap-cgimap/issues/ @westnordost...

There's a reasonable argument that it is rails that is wrong though - the 409 response is meant to mean that the version you gave is out of date because somebody else has edited since you fetched the object. A negative version is just straight up invalid and should arguably be a 400 error as cgimap is doing.

westnordost commented 5 years ago

I created a new issue there and thus will close this one here.

Also, @tomhughes , I agree. I already fixed it in the app and will do an update tomorrow or so. The question is just how much you want to preserve the previous behaviour before the move to cgimap.

mmd-osm commented 5 years ago

Let's take a brief look at the OSM API 0.6 documentation on version numbers:

By using negative version numbers, you're actually violating the second requirement, although the documentation doesn't explicitly mention that an HTTP 400 would be raised in that case.

I somehow tend to backport this behavior to Rails.

As a side note, the diff upload references error messages for individual objects (such as this one), where the docs state on the topic of version numbers:

HTTP status code 400 (Bad Request): When the version of the provided element does not match the current database version of the element

It's interesting that Rails returns HTTP 409 in this case. I haven't looked into it yet to find out where it comes from.

tomhughes commented 5 years ago

It's https://github.com/openstreetmap/openstreetmap-website/blob/master/lib/consistency_validations.rb#L11, which raises APIVersionMismatchError when the versions are different.