On Feb 6, 2013, at 3:33 PM, Sergey Beryozkin <sberyozkin_at_talend.com> wrote:
> On 06/02/13 14:31, 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
>>
>
> Please ignore the above fragment, but I think what I typed below is still readable :-)
>
>> 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 ?
Please understand that "a suspended thread" is an optional implementation detail, which is hidden from the end user. IOW, there may or may not be another suspended thread - depends on your implementation. The AsyncResponse does not represent a suspended thread. AsyncResponse represents a suspended connection. IOW, it is an open socket to the client. For sure the socket is not bare - the hosting I/O container will wrap the socket including additional request-scoped information. (Again, I've never seen an I/O container that wouldn't do that.) So if you invoke a callback on this wrapper and pass it an unmapped exception, you are effectively propagating the exception to the container even though in this case propagate != re-throw. The identity of an actual thread that executes such completion code is irrelevant for our considerations.
>>
>> By the way, I'm not sure what do you mean there's no container in the
>> async case; I'll just add a test and see what happens when the
>> application code resumes AsyncResponse with Throwable which can not be
>> mapped and report back
I'm not sure either. The way I interpret Bill here is that he's just saying that the resume does not happen on the thread managed by the hosting container. Nothing more, nothing less. As I just tried to explain above, I don't think that his statement that "there's no container" would be accurate though. Assuming you are running on top of a lower level I/O container, there always is a container context associated with the suspended connection, you never work with bare sockets.
Marek
>>
>
> Thanks, Sergey