hootenanny icon indicating copy to clipboard operation
hootenanny copied to clipboard

Make translation server more defensive and responsive when it is sent garbage

Open maxgrossman opened this issue 1 year ago • 1 comments

#5616 motivates making translation server more interrogative of payloads so we can start returning responses that are more useful.

We should start with the problem at hand. If a client sends an empty payload, return a 400 response.

Being curious about error handling in general, I tried sending some bad requests and our responses or lack there of might confuse crowds new to hoot.

For example, if I make a get request to /translateFrom and misspell the translation...

curl http://localhost:8094/translateFrom?translation=TDSv701&fcode=AP030&geom=Line`

I'm returned {"error":"Cannot read property 'toOsm' of undefined"}. Might be better to inform the translation is unknown and give a hint to where the translation could be checked, the /translations endpoint.

Or if I were post to /translateTo with valid data but do a similar misspell...

curl -X POST 'http://localhost:8094/translateTo?translation=TDSv701' -d '<osm version="0.6" upload="true" generator="JOSM"><node id="-1" lon="0" lat="0" version="0"><tag k="building" v="yes"/></node></osm>'

We do not even respond! I get curl: (52) Empty reply from server.

First chunk of work is manage the when payload is empty. We can tackle these other problems and see what else we can throw at the service.

maxgrossman avatar Apr 05 '23 14:04 maxgrossman

One thing #5619 revealed is that when the error happens inside one of the hoot binding functions like loadMapFromString, the error does not also lead to a node.js error being thrown.

If i make a request like curl -X POST 'http://localhost:8094/translateTo?translation=TDSv70' -d 'not_xml' I am returned the empty response message you see above.

If I hardcode something I know will throw a node js error into the try catch block like undefined.foo() I get a catch-able generic error and an appropriate 500 status can be replied.

Is this a known behavior of our node.js bindings @bmarchant?

maxgrossman avatar Apr 06 '23 20:04 maxgrossman