james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "robert burrell donkin" <robertburrelldon...@gmail.com>
Subject Re: AbstractJamesHandler error handling
Date Wed, 04 Jul 2007 20:11:49 GMT
On 7/4/07, Stefano Bagnara <apache@bago.org> wrote:
> robert burrell donkin ha scritto:
> > On 7/4/07, Stefano Bagnara <apache@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


Mime
View raw message