search

cyverse-de / terrain

DE's main API entry-point service
Other
4 stars 9 forks source link

CORE-1881, CORE-1883: Fix error handling in NATS connected handlers, fix updating boolean fields #271

Closed johnworth closed 1 year ago

johnworth commented 1 year ago

This one unraveled on me and I ended up having to fix a bunch of stuff.

Error handling wasn't returning a correct error map from NATS handlers in the backend. This should fix that. Additionally, the boolean fields in the maps passed in to update add-ons and subscription add-ons weren't getting set correctly in the update endpoints. This required me to replace the protobuf library with pronto and fix an error in the select-assoc.

JIRA:

slr71 commented 1 year ago

I haven't taken the time to fully understand how the error codes would be translated to HTTP status codes yet, but I didn't spot any obvious problems. From what I can gather so far, it appears that the error codes returned by the subscription service will be something like INTERNAL, NOT_FOUND or MARSHAL_ERROR, for example. If an error is detected in the Clojure code, an exception is thrown with this error code. The exception handlers in Terrain should catch the exception and return the appropriate HTTP status code for the exception.

The piece that I haven't quite worked out yet is that the exception handling code looks for error codes in the format ERR_<desc> (for example, ERR_NOT_FOUND or ERR_BAD_REQUEST). I didn't spot anything in the Subscriptions code or the Terrain (or clojure-commons) code that would add the ERR_ prefix, so I was a little concerned that the HTTP status code would be the default (500) in all cases. I'm probably missing something, though. I have to admit that I haven't been able to spend as much time looking into this as I'd hoped today, and I don't want to hold up development any longer by waiting until I have an answer to approve the PR. 😄