cirruslabs / orchard

Orchestrator for running Tart Virtual Machines on a cluster of Apple Silicon devices
Other
197 stars 17 forks source link

REST API: provide error messages in error responses #66

Closed edigaryev closed 1 year ago

edigaryev commented 1 year ago

Resolves https://github.com/cirruslabs/orchard/issues/53.

edigaryev commented 1 year ago

On the other hand we also introduced the Responder abstraction and this PR embraces it.

That's it, by using Responder we get way more explicit return value/error propagation, which reduces potential errors and makes code clearer.

With ctx.Error() (or ctx.AbortWithError()) things will always be more complicated, as you have not forget about return.

fkorotkov commented 1 year ago

SGTM. Then what do you think of just having body as plain text and not JSON? Or at least renaming NewError to NewErrorResponse?

edigaryev commented 1 year ago

Then what do you think of just having body as plain text and not JSON?

I don't see why we'd want that. JSON will enable for better future extensibility, and parsing it is a no-brainer in most languages.

Or at least renaming NewError to NewErrorResponse?

See 32a93d34068a8cf383e8ea969c9aa3e84237b4a1.