You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by robert burrell donkin <ro...@gmail.com> on 2007/07/03 21:41:48 UTC
AbstractJamesHandler error handling
handleConnection catches RuntimeException but not all throwable
exceptions. whenever an error is thrown this results in a silent
failure. what's the reasoning behind this?
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: AbstractJamesHandler error handling
Posted by Bernd Fondermann <bf...@brainlounge.de>.
robert burrell donkin wrote:
> On 7/4/07, Stefano Bagnara <ap...@bago.org> wrote:
>> robert burrell donkin ha scritto:
>> > handleConnection catches RuntimeException but not all throwable
>> > exceptions. whenever an error is thrown this results in a silent
>> > failure. what's the reasoning behind this?
>> >
>> > - robert
>>
>> It also catches all of the checked exceptions declared by the called
>> code, right? (SocketException, InterruptedIOException, IOException).
>>
>> What kind of "Error" do you think should be catched (and could be
>> succesfully handled) here?
>
> successful handling is a different question. we've argued before about
> specification compliance verses java best practice error handling.
>
> i prefer servers to comply with the specification whenever possible.
> so, in the case of IMAP IMHO JAMES should try to send an untagged BYE
> with a message even when faced with an error.
>
>> E.g: I don't think that catching an OOM in the connection handling is a
>> good idea.
>
> in the event of an OOM exception being thrown, i would expect an
> application to log this fact so that an administrator can do something
> about it. silently dropping connections (which is what happens ATM) is
> IMHO not the right behaviour.
>
> IMHO JAMES should comply with the specification and try to send an
> untagged BYE with an error message before closing the connection. yes,
> this may fail but IMHO at least an attempt should be made.
>
+1
Bernd
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: AbstractJamesHandler error handling
Posted by Stefano Bagnara <ap...@bago.org>.
robert burrell donkin ha scritto:
>> >> E.g: I don't think that catching an OOM in the connection handling
>> is a
>> >> good idea.
>> >
>> > in the event of an OOM exception being thrown, i would expect an
>> > application to log this fact so that an administrator can do something
>> > about it. silently dropping connections (which is what happens ATM) is
>> > IMHO not the right behaviour.
>>
>> I don't see too many problems with dropping the connection: I'm not
>> confident that in an Error condition you should keep managing a
>> conversation (I see there many more possible problems than closing the
>> connection unexpectedly, as clients are written to manage unexpected
>> connections too, as network can fail by definition).
>
> IMAP is an unusual protocol. the server is required to hold the
> connection open for at least 30 minutes after the last client
> communication. IMHO the protocol is suitable only for reliable
> connections and clients don't expect disconnections.
I don't use IMAP too much, but I used it sometime in past on my GSM
connection and galleries made the connection very instable: no big
problems with the IMAP client (Outlook 2000, IIRC). I've been bound to
M$ for many years and now I fortunately choose my MUA and is
Thunderbird, but I only use POP3.
> the protocol states that the server MUST write an untagged BYE before
> closing the connection. i'm an unfan of IMAP but i'd prefer to
> implement the protocol as written (even when wrong).
So if I power off the server then the software is not compliant ?? ;-)
Of course this is interpretation: the MUST is about the software
operations in standard conditions. I bet no server will send the BYE if
I hot-remove the RAM :-P
I just keep in mind that the Error class has a javadoc saying it should
not the cough, and in my real-life experience I had problems with
software trying to catch too much errors: in old JVM catching too much
OOM would result in other more critical virtual machine errors.
>> [...]
>> E.g: sometimes you will end up sending a BYE in the wrong place of the
>> conversation, and this is not good. YOu don't want to send BYE during a
>> message transfer
>
> IMAP is unusual. both client and server are encouraged to talk
> concurrently. client must listen whilst they are transferring a
> message. at the risk of repeating myself, IMHO this isn't good but
> it's what the protocol specifies.
>
>> or during an SSL handshake or anything similar.
>
> IMAP has to be tunneled so AIUI the handshake must have completed
Well, I don't know the IMAP protocol. Take it as an hint: take a
conversation a place a BYE between the text and see if somewhere it
could alter what the Client can think about the server message.
>> So you
>> should track the state to decide whether the BYE is appropriate or not.
>> Maybe some extra work will increase the probability to run also other
>> connections out of memory at the same time.
>
> for IMAP, BYE is appropriate in all states. the rule is simple: write
> untagged BYE before closing the connection. the only grey area would
> be literal data (eg message transfer from the server to the client).
But IMAP is line based, isn't it? What about a BYE in the middle of
another "line" ? Can you ensure this does not happen without wasting
resources?
>> Furthermore you should investigate all the code above the "handler" to
>> see what will happen in case of Error: maybe you'll end up sending a BYE
>> that will never-ever be managed.
>>
>> An Error could also be thrown by a bad bug in the JVM and a wrong access
>> to memory, and you could end up sending wrong data to the client, and
>> so on.
>
> probably the best approach would be to add extra exception (and maybe
> error) handling code within the IMAP handling code where it's aware of
> what's happening.
>
> i'd like to add a system.out log to AbstractJamesHandler followed by a
> rethrow in the event of an error but i'm willing to reconsider if
> people think that this is generally a bad idea.
>
> - robert
My rule of thumb is:
You are the most active developer right now, so what you prefer has more
value than what I prefer (prefer = think is better). I just tell my
preference, if other developers tell they agree with me then you should
stop, otherwise just take or discard any hint at your preference.
In fact, I like to feel free to spread my ideas without pushing my
rules. As Bernd wrote a +1 to your message if I were you I'd simply
proceed with no need to convince me :-)
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: AbstractJamesHandler error handling
Posted by robert burrell donkin <ro...@gmail.com>.
On 7/4/07, Stefano Bagnara <ap...@bago.org> wrote:
> robert burrell donkin ha scritto:
> > On 7/4/07, Stefano Bagnara <ap...@bago.org> wrote:
> >> robert burrell donkin ha scritto:
> >> > handleConnection catches RuntimeException but not all throwable
> >> > exceptions. whenever an error is thrown this results in a silent
> >> > failure. what's the reasoning behind this?
> >> >
> >> > - robert
> >>
> >> It also catches all of the checked exceptions declared by the called
> >> code, right? (SocketException, InterruptedIOException, IOException).
> >>
> >> What kind of "Error" do you think should be catched (and could be
> >> succesfully handled) here?
> >
> > successful handling is a different question. we've argued before about
> > specification compliance verses java best practice error handling.
>
> Don't worry, I was just looking for this explanation ;-)
thought you might be :-)
> > i prefer servers to comply with the specification whenever possible.
> > so, in the case of IMAP IMHO JAMES should try to send an untagged BYE
> > with a message even when faced with an error.
>
> Maybe we should use a custom method for Errors because, IMO, they have a
> completely different meaning.
> This method should be documented to avoid any allocation and limit any
> resource usage to the minimum.
+1
i suspect that IMAP is unusual and that for some (most?) protocols,
just severing the connection would be correct. need to think about the
right approach with regard to subclassing.
> >> E.g: I don't think that catching an OOM in the connection handling is a
> >> good idea.
> >
> > in the event of an OOM exception being thrown, i would expect an
> > application to log this fact so that an administrator can do something
> > about it. silently dropping connections (which is what happens ATM) is
> > IMHO not the right behaviour.
>
> I don't see too many problems with dropping the connection: I'm not
> confident that in an Error condition you should keep managing a
> conversation (I see there many more possible problems than closing the
> connection unexpectedly, as clients are written to manage unexpected
> connections too, as network can fail by definition).
IMAP is an unusual protocol. the server is required to hold the
connection open for at least 30 minutes after the last client
communication. IMHO the protocol is suitable only for reliable
connections and clients don't expect disconnections.
the protocol states that the server MUST write an untagged BYE before
closing the connection. i'm an unfan of IMAP but i'd prefer to
implement the protocol as written (even when wrong).
> The main problem, imho could be the logging, but this is also a delicate
> issue.
>
> About the logging the cornerstone service support ConnectionMonitors
> (and also provide a simple Avalon Logger implementation for this), but
> it only take care of Exceptions and not Errors: cornerstone authors
> probably thought something similar to me.
yes
logging consumes system resources and so may make matters worse
a case could be made for a simple system.out
> > IMHO JAMES should comply with the specification and try to send an
> > untagged BYE with an error message before closing the connection. yes,
> > this may fail but IMHO at least an attempt should be made.
> >
> > - robert
>
> IMHO OOM should not be tried to recover this way, but I can live with
> the catch/log/bye-try.
>
> E.g: sometimes you will end up sending a BYE in the wrong place of the
> conversation, and this is not good. YOu don't want to send BYE during a
> message transfer
IMAP is unusual. both client and server are encouraged to talk
concurrently. client must listen whilst they are transferring a
message. at the risk of repeating myself, IMHO this isn't good but
it's what the protocol specifies.
> or during an SSL handshake or anything similar.
IMAP has to be tunneled so AIUI the handshake must have completed
> So you
> should track the state to decide whether the BYE is appropriate or not.
> Maybe some extra work will increase the probability to run also other
> connections out of memory at the same time.
for IMAP, BYE is appropriate in all states. the rule is simple: write
untagged BYE before closing the connection. the only grey area would
be literal data (eg message transfer from the server to the client).
> Furthermore you should investigate all the code above the "handler" to
> see what will happen in case of Error: maybe you'll end up sending a BYE
> that will never-ever be managed.
>
> An Error could also be thrown by a bad bug in the JVM and a wrong access
> to memory, and you could end up sending wrong data to the client, and so on.
probably the best approach would be to add extra exception (and maybe
error) handling code within the IMAP handling code where it's aware of
what's happening.
i'd like to add a system.out log to AbstractJamesHandler followed by a
rethrow in the event of an error but i'm willing to reconsider if
people think that this is generally a bad idea.
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: AbstractJamesHandler error handling
Posted by Stefano Bagnara <ap...@bago.org>.
robert burrell donkin ha scritto:
> On 7/4/07, Stefano Bagnara <ap...@bago.org> wrote:
>> robert burrell donkin ha scritto:
>> > handleConnection catches RuntimeException but not all throwable
>> > exceptions. whenever an error is thrown this results in a silent
>> > failure. what's the reasoning behind this?
>> >
>> > - robert
>>
>> It also catches all of the checked exceptions declared by the called
>> code, right? (SocketException, InterruptedIOException, IOException).
>>
>> What kind of "Error" do you think should be catched (and could be
>> succesfully handled) here?
>
> successful handling is a different question. we've argued before about
> specification compliance verses java best practice error handling.
Don't worry, I was just looking for this explanation ;-)
> i prefer servers to comply with the specification whenever possible.
> so, in the case of IMAP IMHO JAMES should try to send an untagged BYE
> with a message even when faced with an error.
Maybe we should use a custom method for Errors because, IMO, they have a
completely different meaning.
This method should be documented to avoid any allocation and limit any
resource usage to the minimum.
>> E.g: I don't think that catching an OOM in the connection handling is a
>> good idea.
>
> in the event of an OOM exception being thrown, i would expect an
> application to log this fact so that an administrator can do something
> about it. silently dropping connections (which is what happens ATM) is
> IMHO not the right behaviour.
I don't see too many problems with dropping the connection: I'm not
confident that in an Error condition you should keep managing a
conversation (I see there many more possible problems than closing the
connection unexpectedly, as clients are written to manage unexpected
connections too, as network can fail by definition).
The main problem, imho could be the logging, but this is also a delicate
issue.
About the logging the cornerstone service support ConnectionMonitors
(and also provide a simple Avalon Logger implementation for this), but
it only take care of Exceptions and not Errors: cornerstone authors
probably thought something similar to me.
> IMHO JAMES should comply with the specification and try to send an
> untagged BYE with an error message before closing the connection. yes,
> this may fail but IMHO at least an attempt should be made.
>
> - robert
IMHO OOM should not be tried to recover this way, but I can live with
the catch/log/bye-try.
E.g: sometimes you will end up sending a BYE in the wrong place of the
conversation, and this is not good. YOu don't want to send BYE during a
message transfer or during an SSL handshake or anything similar. So you
should track the state to decide whether the BYE is appropriate or not.
Maybe some extra work will increase the probability to run also other
connections out of memory at the same time.
Furthermore you should investigate all the code above the "handler" to
see what will happen in case of Error: maybe you'll end up sending a BYE
that will never-ever be managed.
An Error could also be thrown by a bad bug in the JVM and a wrong access
to memory, and you could end up sending wrong data to the client, and so on.
If this was a vote I'd write -0, so if no one else share my concern feel
free to proceed with your changes.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: AbstractJamesHandler error handling
Posted by robert burrell donkin <ro...@gmail.com>.
On 7/4/07, Stefano Bagnara <ap...@bago.org> wrote:
> robert burrell donkin ha scritto:
> > handleConnection catches RuntimeException but not all throwable
> > exceptions. whenever an error is thrown this results in a silent
> > failure. what's the reasoning behind this?
> >
> > - robert
>
> It also catches all of the checked exceptions declared by the called
> code, right? (SocketException, InterruptedIOException, IOException).
>
> What kind of "Error" do you think should be catched (and could be
> succesfully handled) here?
successful handling is a different question. we've argued before about
specification compliance verses java best practice error handling.
i prefer servers to comply with the specification whenever possible.
so, in the case of IMAP IMHO JAMES should try to send an untagged BYE
with a message even when faced with an error.
> E.g: I don't think that catching an OOM in the connection handling is a
> good idea.
in the event of an OOM exception being thrown, i would expect an
application to log this fact so that an administrator can do something
about it. silently dropping connections (which is what happens ATM) is
IMHO not the right behaviour.
IMHO JAMES should comply with the specification and try to send an
untagged BYE with an error message before closing the connection. yes,
this may fail but IMHO at least an attempt should be made.
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: AbstractJamesHandler error handling
Posted by Stefano Bagnara <ap...@bago.org>.
robert burrell donkin ha scritto:
> handleConnection catches RuntimeException but not all throwable
> exceptions. whenever an error is thrown this results in a silent
> failure. what's the reasoning behind this?
>
> - robert
It also catches all of the checked exceptions declared by the called
code, right? (SocketException, InterruptedIOException, IOException).
What kind of "Error" do you think should be catched (and could be
succesfully handled) here?
E.g: I don't think that catching an OOM in the connection handling is a
good idea.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org