Hi Hubert,
Thanks for finding the right spec excerpt.
The spec clearly states that aborting the startup is allowed as long
as all subsequent requests result in http 500. IMO this is the ideal
solution. I could do one of two things:
1) change the patch to make the adapter send http 500s for all
requests when a listener fails to initialize
2) forget about it in 1.9 and instead of throwing IAE, log an error
and implement 1) in grizzly 2.0
/i
On Jul 5, 2009, at 5:19 AM, Hubert Iwaniuk wrote:
> Hi Igor,
>
> I like addListener changes except one that throws
> IllegalArgumentException if we failed to create listener instance.
> I think this might be not compliant with spec. Please see https://grizzly.dev.java.net/issues/show_bug.cgi?id=618
> Please verify if I got the spec right.
>
> Cheers,
> Hubert.
>
>
> On Sat, Jul 4, 2009 at 1:39 AM, Igor Minar <iiminar_at_gmail.com> wrote:
> Hi folks,
>
> First of all thanks for the commit rights, I missed your thread on
> grizzly-dev and saw it only recently after JFA told me that I have
> the right already.
>
> Today I finally fixed something I started working on for 1.9.16 and
> unfortunately introduced a regression with my change.
>
> My ultimate goal was to clean up the way listeners are being added
> to an adapter and most importantly make it possible to pass in
> existing listener instances into an adapter.
>
> I'm attaching a patch that addresses these issues:
>
> https://grizzly.dev.java.net/issues/show_bug.cgi?id=618 Exceptions
> from constructors of servlet listeners are being quietly swallowed
> https://grizzly.dev.java.net/issues/show_bug.cgi?id=630 Typesafe api
> for ServletAdapter#addServletListener
> https://grizzly.dev.java.net/issues/show_bug.cgi?id=631 Allow
> instances of ServletContext*Listeners to be added to ServletAdapter
> https://grizzly.dev.java.net/issues/show_bug.cgi?id=666
> ServletAdapter.addServletListener does not accept HttpSessionListener
>
> All of these issues are interconnected, so that's why there is just
> one patch for it.
>
> I made two controversial changes that I'd like to point out and get
> your okay for:
> - if a listener is passed in by name or by class and it is not
> possible to load or initialize it, an IllegalArgumentException is
> thrown. This is to ensure that a misconfigured application fails to
> start rather than start and later on fail to work properly.
> - I deprecated ServletAdapter#addServletListener and replaced it
> with ServletAdapter#addListener (which is overloaded several times).
> This is due to the fact that using this method both servlet and http
> session listeners are being added, so the method was originally
> misnamed.
>
> I didn't want to commit all of this into the trunk in case you don't
> feel comfortable about this change making it into 1.9.17, but I'm
> hopeful that it can be part of 1.9.17 because my project would like
> to use the new typesafe api methods.
>
> All the existing tests are passing + I added 15 new tests just for
> listener loading.
>
> Let me know what you think and if I should go ahead a commit it to
> the trunk.
>
> cheers,
> Igor
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>