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 ?
>
> 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
>
Thanks, Sergey