metaodi / osmapi

Python wrapper for the OpenStreetMap API
http://osmapi.metaodi.ch/
GNU General Public License v3.0
213 stars 41 forks source link

separate error for network failure would be nice #176

Open matkoniecz opened 1 month ago

matkoniecz commented 1 month ago

basically another aspect of #115 #135

It would be nice to have easy way of distinguishing network failures from other issues.

And yes, recovery is dependent on context - as maybe edit was done and maybe was not done. But ability do detect what happened and being to able to catch specifically network failures would be great.

Currently I would need to catch all osmapi.errors.ApiError, cast them to string and try to detect signature comments while hoping for no false positives.

  File "/home/mateusz/.local/lib/python3.10/site-packages/osmapi/OsmApi.py", line 649, in WayUpdate
    return self._do("modify", "way", WayData)
  File "/home/mateusz/.local/lib/python3.10/site-packages/osmapi/OsmApi.py", line 1825, in _do
    return self._do_manu(action, OsmType, OsmData)
  File "/home/mateusz/.local/lib/python3.10/site-packages/osmapi/OsmApi.py", line 1861, in _do_manu
    result = self._session._put(
  File "/home/mateusz/.local/lib/python3.10/site-packages/osmapi/http.py", line 154, in _put
    return self._http("PUT", path, True, data, return_value=return_value)
  File "/home/mateusz/.local/lib/python3.10/site-packages/osmapi/http.py", line 109, in _http
    return self._http_request(
  File "/home/mateusz/.local/lib/python3.10/site-packages/osmapi/http.py", line 81, in _http_request
    raise errors.ApiError(0, str(e), "")
osmapi.errors.ApiError: Request failed: 0 - ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response')) - 
metaodi commented 1 month ago

I see, I'm wondering if there is an easy way to wrap the existing errors (i.e. expose the underlying RequestException to be able to inspect it) or simpls create new exception in this library to cover all? / most? cases. What do you think?

metaodi commented 1 month ago

Just read about the "raise Exception(...) from e" pattern, which seems to be a good idea when wrapping exceptions to pass the original exception to the caller.

So maybe I'll add this to all "wrapped exceptions" while still adding the most common errors as it's own exception class.

matkoniecz commented 1 month ago

If I would implement this I would catch multitude of network exceptions, throw new OsmapiNetworkException or similar.

Maybe include original as a field in a new exception?

No idea is it a good design.