On 2/6/2013 10:09 AM, Marek Potociar wrote:
>
> On Feb 6, 2013, at 3:47 PM, Bill Burke <bburke_at_redhat.com> wrote:
>
>>
>>
>> On 2/6/2013 9:31 AM, Sergey Beryozkin wrote:
>>> On 06/02/13 14:16, Bill Burke wrote:
>>>>
>>>>
>>>> On 2/6/2013 8:47 AM, Sergey Beryozkin wrote:
>>>>> On 06/02/13 13:01, Bill Burke wrote:
>>>>>>
>>>>>>
>>>>>> On 2/5/2013 6:37 PM, Marek Potociar wrote:
>>>>>>>
>>>>>>> On Feb 5, 2013, at 8:59 PM, Bill Burke <bburke_at_redhat.com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2/5/2013 1:53 PM, Marek Potociar wrote:
>>>>>>>>> I was under impression that leakage is implementation detail. We did
>>>>>>>>> not have to specify anything for sync case, why should we specify it
>>>>>>>>> for async case. There's nothing special about async case in this
>>>>>>>>> regard.
>>>>>>>>>
>>>>>>>>
>>>>>>>> In he sync case, the Servlet container is wrapping the JAX-RS
>>>>>>>> invocation. So, exceptions can be propagated and handled by the
>>>>>>>> servlet container if need be. In the async case, a non-servlet thread
>>>>>>>> is handling the response. So, cleanup, exception handling, etc. needs
>>>>>>>> to either be handled by the jax-rs layer, or user code. IMO, cleanup
>>>>>>>> and exception handling should be done in the JAX-RS layer and should
>>>>>>>> be as deterministic.
>>>>>>>
>>>>>>> Servlet is not the only platform JAX-RS runs on. Anyway, I think that
>>>>>>> propagating the exception to the IO container is deterministic enough.
>>>>>>> Whether or not this happens on the original thread is IMO orthogonal.
>>>>>>> All IO containers that support async externalise all the necessary
>>>>>>> connection-related context in one way or another and always provide a
>>>>>>> facility for propagating exceptions back to this context. I have yet
>>>>>>> to see a container that does not do that.
>>>>>>>
>>>>>>>>
>>>>>>>>> That said, I'm fine with adding a short sentence to the
>>>>>>>>> AR.resume()/cancel() javadoc stating that "A successful invocation
>>>>>>>>> of this method eventually completes the suspended request
>>>>>>>>> processing." or similar. FWIW, I do not want to write anything like
>>>>>>>>> "when this method completes, the request processing is complete",
>>>>>>>>> because I do not want to place restrictions on implementations. E.g.
>>>>>>>>> implementors may decide to complete the response processing
>>>>>>>>> asynchronously, which means that when the method returns the
>>>>>>>>> connection does not have to be necessarily closed yet. Which is also
>>>>>>>>> why I think it's better to not say anything...
>>>>>>>>>
>>>>>>>>
>>>>>>>> Yeah, that's ok. If you want to force synchronicity, then you can
>>>>>>>> just do it in the callback layer. It just wasn't very clear in the
>>>>>>>> javadoc what was supposed to happen with unmapped exceptions. In the
>>>>>>>> sync layer, the unmapped exception is rethrown. You just can't do
>>>>>>>> that in the async layer, IMO.
>>>>>>>
>>>>>>> Yes, that's the main difference. While in sync case the propagation is
>>>>>>> achieved by re-throwing and automatic rollback of the call stack, in
>>>>>>> async case this propagation needs to happen in some other way. Yet
>>>>>>> again, I have not seen any async-enabled IO container that would not
>>>>>>> expose such facility.
>>>>>>>
>>>>>>
>>>>>> Again, I think you are incorrect. If you do not have explicit language
>>>>>> then the user does not know if they have to wrap resume() calls in
>>>>>> try/catch blocks and call cancel. The current language of the javadoc
>>>>>> makes it seem like unmapped exceptions are thrown from resume() and
>>>>>> that
>>>>>> the user needs to handle this. Which is why I started this thread in
>>>>>> the
>>>>>> first place...
>>>>>>
>>>>>> IMO, at least the javadoc should change from:
>>>>>>
>>>>>> "The processing of the data by JAX-RS framework follows the same
>>>>>> path as
>>>>>> * it would for the response data returned synchronously by a JAX-RS
>>>>>> resource
>>>>>> * method."
>>>>>>
>>>>>> To:
>>>>>>
>>>>>> "By executing this method, the request is guaranteed to be completed is
>>>>>> some form or another. The processing of the data by the JAX-RS
>>>>>> framework
>>>>>> follows the same path as it would for the response data returned
>>>>>> synchronously by a JAX-RS resource except that unmapped exceptions are
>>>>>> not propagated. Depending on the JAX-RS implementation, unmapped
>>>>>> exceptions will result in an error status being sent to the client
>>>>>> and/or the connection being closed."
>>>>>>
>>>>>> I think the above gives jax-rs implementations enough flexibility, but
>>>>>> makes it clear to the user how to interact with resume()/cancel().
>>>>>>
>>>>> While I can not say I always agree with Marek, but the current version
>>>>> seems quite precise to me. Even in the sync case, the unmapped exception
>>>>> will result in 500 being sent (by the container, on the servlet thread),
>>>>> so why distinguish how this 500 may end up being reported by the
>>>>> container in the async case ?
>>>>>
>>>>
>>>> Maybe clear to you, but not clear to me. The resume() javadoc states
>>>> that execution works the same as the sync case. If you read spec section
>>>> 3.3.4 on Exceptions, bullet #4 states that unmapped and unhandled
>>>> exceptions are wrapped and allowed to propagate to the container. There
>>>> *IS* no container to propagate to in the async case. So, this leads me
>>>> to believe that you have to have try/catch blocks around resume() calls.
>>>>
>>>> I think unmapped exceptions should be handled by resume() and cancel()
>>>> by either sending an error code back to the client or closing the
>>>> connection. This should be explicitly stated, or you'll have users like
>>>> me misinterpreting the javadoc and having try/catch blocks around our
>>>> resume() methods.
>>>>
>>> Hmm... So if the user resumes an invocation, by explicitly calling on
>>> AsyncResponse:
>>>
>>> asyncResponse.resume()
>>>
>>> I'm presuming it is going to happen in some application thread,
>>> so then the suspended thread will return to some function in the
>>> application code, it throws the exception, right, so how come
>>> one can expect to do
>>>
>>> try {
>>> asyncResponse.resume(myObject)
>>> } catch (Throwable t) {
>>> }
>>>
>>> ?
>>>
>>> In other words,
>>>
>>> 1. Thread comes in and calls myJaxrsResourceMethod(@Suspended
>>> AsyncResponse) and the application code just saves AsyncResponse in some
>>> pool.
>>>
>>> 2. Next, the user code somehow resumes this AsyncResponse, all within
>>> the application code, and the suspended thread returns, picks the
>>> response Object up from AsyncResponse (with the help of the runtime),
>>> without even entering the application code again, and proceeds same way
>>> as it is done in the sync case.
>>>
>>> How do you expect the catch block in the above code be executed ?
>>>
>>> By the way, I'm not sure what do you mean there's no container in the
>>> async case;
>>
>> I think Marek finally agreed with me, but I'll try to convince you too Sergey.
>
> No, I did not. IMO there always is a container. What you perhaps wanted to point out that the resume may happen on a thread that is not managed by container and with a call-stack that does not originate in a container service call. Which does not mean that there is no container in async case though.
>
There's no container to propagate the unmapped exception to in the async
case because an application thread is wrapping the call around resume().
This is what I meant by 'no container'.
Also, 'propagate' to me has always meant rethrow.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com