<p dir="ltr">The code you highlighted comes from the unit tests. That's why it seems too generic.<br>
If you look at the actual controller class code, you will notice we are catching several kinds of exceptions and doing what needs to be done on each. But we still have the possibility that another exception we didn't consider reached the controller, hence the 'Something went wrong, see the logs' generic message. It's a catch-all fallback.</p>
<p dir="ltr">Regarding the error codes, we have followed what seemed the regular use in bibliography, which is of course subject to discussion/improvement. But keep in mind that it is mostly bad formed data, or non existing fields attempted to be updated/created, so I think the 500 is a good idea. What error you think would suit better?</p>
<p dir="ltr">BTW, Martin highlighted yesterday that we could make heavier use of swagger for parameters validation, so in theory we should not hit the controller script (not to speak of dbix) if parameters are wrong. I have the feeling we shouldn't, because the trade off in terms of bureaucracy on writing json files is quite negative.</p>
<p dir="ltr">Anyway, I still think it makes a whole lot of sense that we raise an exception on Koha::Objects if dbix would die due to bad parameters, don't you think?</p>
<br><div class="gmail_quote"><div dir="ltr">El jue., 13 de oct. de 2016 2:08 AM, David Cook <<a href="mailto:dcook@prosentient.com.au">dcook@prosentient.com.au</a>> escribió:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Cheers, Fridolin!<br class="gmail_msg">
<br class="gmail_msg">
In terms of CRUD endpoints, I wonder how it would work for end users... looking at <a href="https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428" rel="noreferrer" class="gmail_msg" target="_blank">https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428</a>, the messages seem a bit strange to me.<br class="gmail_msg">
<br class="gmail_msg">
{ error => "Something went wrong, check the logs." }, 500 );<br class="gmail_msg">
<br class="gmail_msg">
That might make sense if it's just Koha consuming Koha services, but what about third-parties?<br class="gmail_msg">
<br class="gmail_msg">
I imagine it more like the Spotify API:<br class="gmail_msg">
<a href="https://developer.spotify.com/web-api/get-album/" rel="noreferrer" class="gmail_msg" target="_blank">https://developer.spotify.com/web-api/get-album/</a><br class="gmail_msg">
<a href="https://developer.spotify.com/web-api/user-guide/#response-status-codes" rel="noreferrer" class="gmail_msg" target="_blank">https://developer.spotify.com/web-api/user-guide/#response-status-codes</a><br class="gmail_msg">
<a href="https://developer.spotify.com/web-api/user-guide/#error-details" rel="noreferrer" class="gmail_msg" target="_blank">https://developer.spotify.com/web-api/user-guide/#error-details</a><br class="gmail_msg">
<br class="gmail_msg">
You return the HTTP code and then you have a standard format for details. With a 500 error, I don't think you'd actually send a message, since you don't really have anything to say, since it's an internal server error.<br class="gmail_msg">
<br class="gmail_msg">
Actually, I'm not sure about  the use of 500 for DBIx::Class::Exception... but I haven't looked into our use of that exception yet. If you've provided a bad value or a bad field, maybe it would make more sense to return 400 for a bad request rather than saying there was an internal server error which tend to arise unexpectedly. Bad values or bad fields are somewhat predictable.<br class="gmail_msg">
<br class="gmail_msg">
Anyway, I'm just curious about whatever conventions we use, so that I can apply them consistently in whatever code I (and others) contribute.<br class="gmail_msg">
<br class="gmail_msg">
David Cook<br class="gmail_msg">
Systems Librarian<br class="gmail_msg">
Prosentient Systems<br class="gmail_msg">
72/330 Wattle St<br class="gmail_msg">
Ultimo, NSW 2007<br class="gmail_msg">
Australia<br class="gmail_msg">
<br class="gmail_msg">
Office: 02 9212 0899<br class="gmail_msg">
Direct: 02 8005 0595<br class="gmail_msg">
<br class="gmail_msg">
> -----Original Message-----<br class="gmail_msg">
> From: <a href="mailto:koha-devel-bounces@lists.koha-community.org" class="gmail_msg" target="_blank">koha-devel-bounces@lists.koha-community.org</a> [mailto:<a href="mailto:koha-devel-" class="gmail_msg" target="_blank">koha-devel-</a><br class="gmail_msg">
> <a href="mailto:bounces@lists.koha-community.org" class="gmail_msg" target="_blank">bounces@lists.koha-community.org</a>] On Behalf Of Fridolin SOMERS<br class="gmail_msg">
> Sent: Wednesday, 12 October 2016 6:32 PM<br class="gmail_msg">
> To: <a href="mailto:koha-devel@lists.koha-community.org" class="gmail_msg" target="_blank">koha-devel@lists.koha-community.org</a><br class="gmail_msg">
> Subject: Re: [Koha-devel] Marseille16 / REST api<br class="gmail_msg">
><br class="gmail_msg">
> Here are my notes :<br class="gmail_msg">
> <a href="https://wiki.koha-" rel="noreferrer" class="gmail_msg" target="_blank">https://wiki.koha-</a><br class="gmail_msg">
> <a href="http://community.org/wiki/Marseille_hackfest_2016#REST_API_-_Thomas" rel="noreferrer" class="gmail_msg" target="_blank">community.org/wiki/Marseille_hackfest_2016#REST_API_-_Thomas</a><br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> Le 12/10/2016 à 09:17, Tomas Cohen Arazi a écrit :<br class="gmail_msg">
> > El mié., 12 oct. 2016 a las 7:26, David Cook<br class="gmail_msg">
> > (<<a href="mailto:dcook@prosentient.com.au" class="gmail_msg" target="_blank">dcook@prosentient.com.au</a>>)<br class="gmail_msg">
> > escribió:<br class="gmail_msg">
> ><br class="gmail_msg">
> >> Really curious to see how this plays out!<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> I’ve just added parameter validation to a Koha::Object, and while it<br class="gmail_msg">
> >> works well enough, I’m hoping for a more generalized solution. I<br class="gmail_msg">
> >> haven’t looked at Koha::Exceptions::* yet, but maybe that will help.<br class="gmail_msg">
> >><br class="gmail_msg">
> ><br class="gmail_msg">
> > I don't know how that would work, what bug number is it?<br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> >> Peeking at Koha::Exceptions::MissingParameter, I hope that there’s<br class="gmail_msg">
> >> more specificity than just “A required parameter is missing”. I’d<br class="gmail_msg">
> >> want to know which required parameter is missing.<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> Is there a document somewhere saying how to use Koha::Exceptions?<br class="gmail_msg">
> >><br class="gmail_msg">
> ><br class="gmail_msg">
> > Well, we are not baking our own exception system, we are just using<br class="gmail_msg">
> > Class::Exception so it is pretty documented!<br class="gmail_msg">
> ><br class="gmail_msg">
> > Look at bug 17425, you will notice when we throw an exception we can<br class="gmail_msg">
> > add a message.<br class="gmail_msg">
> ><br class="gmail_msg">
> > Regards!<br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> >><br class="gmail_msg">
> >> David Cook<br class="gmail_msg">
> >><br class="gmail_msg">
> >> Systems Librarian<br class="gmail_msg">
> >><br class="gmail_msg">
> >> Prosentient Systems<br class="gmail_msg">
> >><br class="gmail_msg">
> >> 72/330 Wattle St<br class="gmail_msg">
> >><br class="gmail_msg">
> >> Ultimo, NSW 2007<br class="gmail_msg">
> >><br class="gmail_msg">
> >> Australia<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> Office: 02 9212 0899 <02%2092%2012%2008%2099><br class="gmail_msg">
> >><br class="gmail_msg">
> >> Direct: 02 8005 0595 <02%2080%2005%2005%2095><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> *From:* <a href="mailto:koha-devel-bounces@lists.koha-community.org" class="gmail_msg" target="_blank">koha-devel-bounces@lists.koha-community.org</a> [mailto:<br class="gmail_msg">
> >> <a href="mailto:koha-devel-bounces@lists.koha-community.org" class="gmail_msg" target="_blank">koha-devel-bounces@lists.koha-community.org</a>] *On Behalf Of *Tomas<br class="gmail_msg">
> >> Cohen Arazi<br class="gmail_msg">
> >> *Sent:* Wednesday, 12 October 2016 2:39 AM<br class="gmail_msg">
> >> *To:* koha-devel <<a href="mailto:koha-devel@lists.koha-community.org" class="gmail_msg" target="_blank">koha-devel@lists.koha-community.org</a>><br class="gmail_msg">
> >> *Subject:* [Koha-devel] Marseille16 / REST api<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> During the first couple Hackfest days, Jonathan, Benjamin and I<br class="gmail_msg">
> >> worked on a reference implementation of CRUD code for a REST<br class="gmail_msg">
> >> endpoint, so others can read it, discuss it and use it as a reference for<br class="gmail_msg">
> their own developments.<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> We worked on Koha::Cit(y|ies) because it was a really simple class.<br class="gmail_msg">
> >> All our work is now on bug 17428.<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> The main goals/concepts we tried to apply were:<br class="gmail_msg">
> >><br class="gmail_msg">
> >> * Controller class (Koha::REST::V1::Cities) should be as simple as<br class="gmail_msg">
> >> possible. No business logic on it. All business logic is expected to<br class="gmail_msg">
> >> be put on the Koha::Cit(ies|y) classes. And fully tested.<br class="gmail_msg">
> >><br class="gmail_msg">
> >> * All verbs/use cases should be covered by tests<br class="gmail_msg">
> >> (t/db_dependent/api/v1/cities.t). We did it, and this file could be<br class="gmail_msg">
> >> used (with little adjustments) as the basis for new endpoint tests.<br class="gmail_msg">
> >><br class="gmail_msg">
> >> * Heavy use of exceptions. I wouldn't say we heavily use them, but<br class="gmail_msg">
> >> the point is that we pass everything to the business object, and<br class="gmail_msg">
> >> catch exceptions it might raise. Martin pointed out that he would<br class="gmail_msg">
> >> help us further simplify it, making heavier use of the Swagger plugin's<br class="gmail_msg">
> facilities.<br class="gmail_msg">
> >> Specially on parameters sanity check. More on this tomorrow!<br class="gmail_msg">
> >><br class="gmail_msg">
> >> * Koha::Exceptions got per-class files so the exceptions file is more<br class="gmail_msg">
> >> manageable. Bug 17425 introduced Koha/Exceptions/Object.pm, and we<br class="gmail_msg">
> >> propose that's the way to go (Koha/Exceptions.pm still has<br class="gmail_msg">
> >> Virtualshelves-related exceptions that should be moved).<br class="gmail_msg">
> >><br class="gmail_msg">
> >> * Make use of Try::Tiny for try/catch blocks. On the process we wrote<br class="gmail_msg">
> >> bug 17425, which introduces new exceptions, we added the dependency<br class="gmail_msg">
> >> for Try::Tiny (no worries, it is packaged for Jessie/Ubuntu) and<br class="gmail_msg">
> >> Mirko was here to validate.<br class="gmail_msg">
> >><br class="gmail_msg">
> >> * Moving business-logic to Koha:: namespace and write REST endpoints<br class="gmail_msg">
> >> on top of that business objects. We picked a simple object,<br class="gmail_msg">
> >> Koha::Patron(s) will be a more complex one of course. This is what<br class="gmail_msg">
> >> Jonathan has been doing, and people willing to write REST endpoints<br class="gmail_msg">
> >> should check with him, and probably help moving business logic into<br class="gmail_msg">
> Koha:: namespace.<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> There are several patches on bugzilla introducing endpoints. We will<br class="gmail_msg">
> >> try to contact patch authors to help them make their work match this<br class="gmail_msg">
> >> proposed 'guidelines'.<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> We are sticking to the 'patches speak' approach, so feel free to<br class="gmail_msg">
> >> criticize this implementation, to improve it or even provide a<br class="gmail_msg">
> >> counter-proposal. We are otherwise moving on, trying to get a functional<br class="gmail_msg">
> complete REST api ASAP.<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> Thanks!<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> Tomas, Jonathan, Benjamin<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> --<br class="gmail_msg">
> >><br class="gmail_msg">
> >> Tomás Cohen Arazi<br class="gmail_msg">
> >><br class="gmail_msg">
> >> Theke Solutions (<a href="https://theke.io" rel="noreferrer" class="gmail_msg" target="_blank">https://theke.io</a> <<a href="http://theke.io/" rel="noreferrer" class="gmail_msg" target="_blank">http://theke.io/</a>>) ✆ +54 9351<br class="gmail_msg">
> >> 3513384 <+54%209%20351%20351-3384><br class="gmail_msg">
> >> GPG: B2F3C15F<br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >><br class="gmail_msg">
> >> _______________________________________________<br class="gmail_msg">
> >> Koha-devel mailing list<br class="gmail_msg">
> >> <a href="mailto:Koha-devel@lists.koha-community.org" class="gmail_msg" target="_blank">Koha-devel@lists.koha-community.org</a><br class="gmail_msg">
> >> <a href="http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel</a><br class="gmail_msg">
> >> website : <a href="http://www.koha-community.org/" rel="noreferrer" class="gmail_msg" target="_blank">http://www.koha-community.org/</a> git :<br class="gmail_msg">
> >> <a href="http://git.koha-community.org/" rel="noreferrer" class="gmail_msg" target="_blank">http://git.koha-community.org/</a> bugs : <a href="http://bugs.koha-community.org/" rel="noreferrer" class="gmail_msg" target="_blank">http://bugs.koha-community.org/</a><br class="gmail_msg">
><br class="gmail_msg">
> --<br class="gmail_msg">
> Fridolin SOMERS<br class="gmail_msg">
> Biblibre - Pôles support et système<br class="gmail_msg">
> <a href="mailto:fridolin.somers@biblibre.com" class="gmail_msg" target="_blank">fridolin.somers@biblibre.com</a><br class="gmail_msg">
> _______________________________________________<br class="gmail_msg">
> Koha-devel mailing list<br class="gmail_msg">
> <a href="mailto:Koha-devel@lists.koha-community.org" class="gmail_msg" target="_blank">Koha-devel@lists.koha-community.org</a><br class="gmail_msg">
> <a href="http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel</a><br class="gmail_msg">
> website : <a href="http://www.koha-community.org/" rel="noreferrer" class="gmail_msg" target="_blank">http://www.koha-community.org/</a> git : <a href="http://git.koha-" rel="noreferrer" class="gmail_msg" target="_blank">http://git.koha-</a><br class="gmail_msg">
> <a href="http://community.org/" rel="noreferrer" class="gmail_msg" target="_blank">community.org/</a> bugs : <a href="http://bugs.koha-community.org/" rel="noreferrer" class="gmail_msg" target="_blank">http://bugs.koha-community.org/</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div><div dir="ltr">-- <br></div><div data-smartmail="gmail_signature"><div dir="ltr"><div style="color:rgb(117,117,117);font-family:"helvetica neue",helvetica,arial,sans-serif;font-size:12.8px">Tomás Cohen Arazi</div><div style="color:rgb(117,117,117);font-family:"helvetica neue",helvetica,arial,sans-serif;font-size:12.8px">Theke Solutions (<a href="http://theke.io/">https://theke.io</a>)<br>✆ +54 9351 3513384<br>GPG: B2F3C15F</div></div></div>