> We do seem to be going around in circles a bit :-)
Part of the reason for that is - I guess - that we both don't have an
all that clear understanding of the problem nor the solution, so I
think that's ok.
> I guess i am having a hard time understanding why in all cases the commit or
> rollback has to be performed after the response has been serialized. And, as
> you can tell i lack a certain experience in the area :-)
Well, you just might need a transaction to write the response. For
example, many O/R mappers support lazy loading, so if you access
certain properties of your mapped objects during result serialization,
you'll get errors if the transaction is already closed.
In my use case, I'm writing code against our own native XML database
(EMC xDB, formerly X-Hive/DB). When you serialize a large document or
BLOB there, you certainly don't want to buffer it, commit, and then
write the response but rather stream it out directly. Again, you'll
need an open transaction for that.
This of course might not apply for error cases where you probably
don't need further access to the database (or do you? maybe for
logging the error?).
> So for that case the application could do one of the following:
>
> 1) return a 412, e.g. return Response.status(412).build().
>
> 2) throw a WebApplicationException(412)
>
> 3) or throw an exception that is mapped to 412.
>
> in addition to a general case of:
>
> 4) or thrown an exception that is unmapped and passed to the container
>
> The common factor, for the first three, is you want on a 412 status code to
> perform rollback. In addition rollback should be performed for 4) and for
> the case of an exception thrown that is mapped to a 5xx response.
See below.
> I think you mentioned this before, but what if the application could signal
> that a set of status codes from a resource method should result in a
> rollback ?
I wouldn't really be happy with that. The meaning of a status code or
how it happened is specific for a certain method, e.g. in method A if
you end up with a 412 it means you need to roll back, but in method B
in a larger application it might not mean the same. Again, status
codes are part of the public interface of the RESTful service, but the
internal implementation might be quite different - I think there is
just no one-to-one mapping.
>> You could either have
>> a dedicated "ExceptionListener" or extend the ExceptionMapper somehow,
>> but the first one is probably cleaner. Is there anything that speaks
>> against this?
>
> I am still not sure that is sufficient for all the cases of how a response
> can be returned.
>
> However, do you propose that, given the pre-condition example above that
> cases 2, 3, 4 should result in rollback, but not 1)?
I'd propose that we retain the semantics of contained managed
transactions with servlets, so that if an unchecked exception is
thrown (which somehow translates to an unexpected exception), a
rollback should occur.
I'd argue that the WebApplicationExceptions aren't really unexpected
exceptions, as user code explicitly signals some condition with it -
it's more like a special return value. So in the place where the user
code throws the exception, it can easily rollback or commit on it's
own, or signal it to the container somehow. So here I would commit a
transaction. So 1) and 2) should result in a commit.
Just passing through the exception to the container (4) works as is.
But that's a bit of a sad exception, as users will then just run into
a standard Jetty, Tomcat, or whatever stack trace. It would be nice to
be able to use an ExceptionMapper there.
So I'd like to use an ExceptionMapper, and still be able to reliably
do a rollback. The simple solution to that is just manually perform
the rollback in the code for the ExceptionMapper, but this is easy to
forget, and the code that initially opened the transaction is not
involved in it.
> I think we could have the following state on a response:
>
> a) for an exception mapped to a response we could set the response state to
> stay
> MAPPED_EXCEPTION.
>
> b) for an umapped exception passed to the container, we could set the
> response state
> to say UNMAPPED_EXCEPTION
>
> c) otherwise the state is DIRECT_RESPONSE
>
> Then in the Closeable the response state could be checked.
>
> Is that something that would work for you?
Yes, that would work, at least if WebApplicationExceptions are a case
for DIRECT_RESPONSE. Basically I need to know if execution flow went
through one of the "catch (RuntimeException e)" blocks in
WebApplicationImpl.
Somewhat related: there is no catch block around "response.write()" in
that place. What happens if a streaming output throws a
RuntimeException? The spec (1.1 draft) doesn't seem to cover that.
Martin