Re: Proposal to differentiate between program and processing(precondition) errors

[Chris Travers <..hidden..>]

Hi John;

Good thoughts!

On Sun, Jun 21, 2015 at 10:29 PM, John Locke <..hidden..> wrote:
> One more time!
>
>
> On 06/21/2015 01:41 PM, Erik Huelsmann wrote:
>>
>> Currently, all errors generated by LedgerSMB are being raised through
>> 'die'. It's an effective way, but my feeling is that it bleeds through
>> too many details of the internals. This is especially true when the
>> error is really nothing more than a reported "missing configuration".
>>
>> E.g. we currently generate an error in case there's a field's value
>> missing. The generated error includes a stack dump and all!
>
> This does seem absurd at this point -- why not just change those to
> throw exceptions? Then we can catch them at the appropriate point and
> bubble those up to the interface.

In Perl the difference between croak and die is saying "you gave me
bad data" and "I messed up."

I think we would do well to at least allow a difference between a 4xx
and a 5xx error to provide something similar.  I have a patch to
commit tonight to do that.  Preprocessing errors should be 4xx errors,
processing and post-processing errors should be 5xx errors, I think.
This allows service consumers to determine how to handle based on the
status.

But this leads to some issues.  A required field that is missing from
a db entry should be a 5xx error but a required field missing from a
field on request is a 4xx error.
>
>>
>> My proposal is that in this type of error case, we're checking the
>> existence of the required fields and their values. If these don't
>> exist, we should be reporting a nicely formatted error to the client
>> -- most definitely without a stack trace.
>>
>> Also, the HTTP status code for each error-with-stacktrace is currently
>> "500" -- Internal Server Error. When we *understand* the request, but
>> can't process it, we should be generating a 422 (Unprocessable entity)
>> response or alike.
>>
>>
>> Now for the question: what would be the best way to achieve this? (I
>> think this applies equally to HTML page responses as it does to web
>> service api calls, so this is a general question that needs a general
>> solution which is applicable for a long time.)
>
> Exceptions. Without question.

Right now that's what we are using.   I don't see us moving away from
them.  They are convenient, if quick and dirty.  They are generally
accepted in the Perl community (we aren't programming in, say, Scala
where referential transparency is a lot more important).  However I
could see an argument for moving away from them for preprocessing
conditions.  See below in discussion of response objects.
>
> We do need separate response objects that get instantiated right at the
> start of the request. These should have some sort of ending method --
> something like ->send() -- to actually output the response back to the
> requestor, in the appropriate format.

Ok, so in 1.5 we effectively only support Plack.  We pretend to do
this via CGI but the code is moving to a point where we could very
easily change this.  In other words right now we "print" but that
actually goes to a wrapper, which returns the Plack response, which
then gets converted back to whatever gateway interface is actually
happening behind the scenes.  PSGI servers would just convert to HTTP
responses.  CGI environmnets would print out things in relevant ways,
etc.  So far so good.

The fundamental assumption of Plack is that a response is a function
of the request.  Eventually we should be getting to a point where
instead of printing a response, we simply return it.  Exceptions
should go to an exception handler that returns a response.

In this view, I think we'd do best to do something like:

my $validation_err = LedgerSMB::Request->requires('foo', 'bar', 'baz');
return $validation_err if $validation_err;

This would allow some additional things that exceptions wouldn't
allow.  For example, we could check *all* required fields and return a
full set of them rather than dying on first error.  Now, I know this
could still be done with exceptions (.e. delaying them and then dying
later) but I think in that case, this may be cleaner.  One could also
allow for multiple validation checks and errors to be merged and so
forth (which exceptions can't readily do since by definition they
break program flow unless we put them in eval{} )..

If $validation_err implements a response interface, then we are in
good shape.  We could also handle CLI tools by making our exit codes
for non-2xx/3xx responses into the http status codes.

>
> Server side, we need a global exception handler to catch anything not
> caught during actual processing, which can attach the appropriate error
> data to the response header along with the appropriate error code. The
> response object should generally return an appropriate http response
> code, and can then pass the stack trace (if appropriate) and any other
> data necessary, on up to the browser.

Yep, we have that, or rather we have 2, one for old code and one for new code.
>
> The response object should be smart enough to know how much detail to
> transmit -- perhaps some other service can filter messages if we want to
> keep very sensitive stuff away from the browser, and only transmit it
> all if the site is set to 'debug'...

Actually the problem in handling Moose instantiation and stack traces
goes a bit deeper.

The stack traces are appended to the exception text by Moose, so my
thinking is we want to avoid sending those at all back to the browser.
I think we are much better off with a wrapper that can be told "we are
getting this from the database and not the UI" and appropriately
transform user-visible error messages in that context.  Again Perl has
some useful features in this regard.

But basically we need to catch the exception, process it in the
context we are actually running at the moment, and then then handle
that.  That's doable, but just needs some thought.  We don't have too
many places we need to do that so it should be possible at least for
new code.

>
>
> So really what I've described so far is how to get an exception up to
> the browser. We haven't really handled it, just transmitted it. If we're
> going single page, we really need to handle that in the browser, so most
> of the actual graceful handling of the exceptions will end up being
> Javascript.

Right and to my mind that means being able to receive errors in html
or json, and being able to differentiate based on http status.
>
>
> I think one area we might want to explore further is what kinds of
> exceptions we want to handle, where. Do we need specialized exception
> classes? Do we catch some kinds of exceptions and simply log them, then
> resume processing?

100% agreed here.
>
> Cheers,
> John
> http://www.freelok.com
>
>
> ------------------------------------------------------------------------------
> Monitor 25 network devices or servers for free with OpManager!
> OpManager is web-based network management software that monitors
> network devices and physical & virtual servers, alerts via email & sms
> for fault. Monitor 25 devices for free with no restriction. Download now
> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
> _______________________________________________
> Ledger-smb-devel mailing list
> ..hidden..
> https://lists.sourceforge.net/lists/listinfo/ledger-smb-devel

------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors 
network devices and physical & virtual servers, alerts via email & sms 
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
Ledger-smb-devel mailing list
..hidden..
https://lists.sourceforge.net/lists/listinfo/ledger-smb-devel