Salut,
Hubert Iwaniuk wrote:
> Hi,
>
> We could also check how other containers behave in listener init fail
> situation.
> If Servlet TCK will pass with behavior other than from Spec but same as
> other containers, I'd vote for same as others.
>
> Also apart from reporting 500 on invalid listener resource, including
> message explaining why we did 500 would be nice ("Listener 'name' failed
> to init with: 'e.getMessage()'."), and potentialy helpful for debugging.
+1
I would propose we send 503 instead of 500. 503 means unavailable. That
will be much simpler IMO to understand for a developer.
A+
--Jeanfrancois
>
> Great work!
>
> Cheers,
> Hubert.
>
>
> On Mon, Jul 6, 2009 at 5:45 AM, Igor Minar <iiminar_at_gmail.com
> <mailto:iiminar_at_gmail.com>> wrote:
>
> 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
> <mailto: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
> <mailto:dev-unsubscribe_at_grizzly.dev.java.net>
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
> <mailto:dev-help_at_grizzly.dev.java.net>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> <mailto:dev-unsubscribe_at_grizzly.dev.java.net>
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
> <mailto:dev-help_at_grizzly.dev.java.net>
>
>