search

heremaps / xyz-hub

XYZ Hub is a RESTful web service for the access and management of geospatial data.
Apache License 2.0
66 stars 39 forks source link

Consider using standard http error code instead of 513 #958

Closed olifink closed 1 year ago

olifink commented 1 year ago

hey all!

maybe a long standing typo, but using custom http errors makes creating code stubs from swagger almost impossible. you should change your custom 513/payload too large to standard http 413 Payload Too Large

https://github.com/heremaps/xyz-hub/blob/a7eebb1756ae0a28c805ea445c9e061303d24eb9/xyz-hub-service/src/main/java/com/here/xyz/hub/rest/Api.java#L91

would also make more sense to have it as as 4xx client and not a 5xx server error

O

roegi commented 1 year ago

Hi @olifink :)

Thanks for finding / pointing this out. Actually back then when we introduced it, it was not a typo, but we wanted to distinguish a "too large response" from a "too large request" payload error (413 Payload Too Large is referring to the request entity). I agree, that it's actually not nice that it's a server error. It's a kind of in a grey zone, because it's not the user's fault that we're not offering paging / iteration on some resources (e.g. /tiles), but actually a 4xx makes more sense, because in these cases only the user can change the query to reduce the expected size of the response-payload.

Would it help already to change it to some other 4xx custom error code (to still make it possible for the user to distinguish between a request- vs response-payload being to large) or does it have to be 413 to make the the code stub generation possible? I think in the latter case we could still think about slightly "miss-using" the 413, but still have the descriptive error message in the payload which explains the actual cause.

Ben

olifink commented 1 year ago

Hey Ben 👋

yeah, I thought that might be the case. The non-standard response codes (513, 499, ...) are certainly one issue that makes wrapper generators fall flat on their faces, but it might not even be the only one.

Swagger CodeGen runs into some NPE, others don't handle OAS3 yaml and there was only one I could work by ignoring errors. Even then it did include sections as sub-packages.

Soo, I will probably manually re-write a yaml so that it's not elegant but good enough for generators because writing client stubs by hand is even less fun... 😵‍💫

O