Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VaadinServlet won't initialize under specific circumstances #20472

Open
mvysny opened this issue Nov 14, 2024 · 4 comments
Open

VaadinServlet won't initialize under specific circumstances #20472

mvysny opened this issue Nov 14, 2024 · 4 comments

Comments

@mvysny
Copy link
Member

mvysny commented Nov 14, 2024

Description of the bug

Under certain circumstances (listed below), Vaadin isn't initialized properly and doesn't set the Lookup instance to the session. That causes VaadinServlet to return prematurely from the initialization, but via a return statement instead of throwing an exception (see VaadinServlet:126, the vaadinServletContext.getAttribute(Lookup.class) == null statement). That causes the servlet container to believe that VaadinServlet has initialized and is ready to serve the requests, but the Servlet is not ready: the staticFileHandler is null, which causes any request for static files to fail with a NullPointerException: Cannot invoke "com.vaadin.flow.server.StaticFileHandler.serveStaticResource(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)" because "this.staticFileHandler" is null.

Related tickets: #15991 and #10238

This happens when Vaadin app is starting in an embedded Jetty with classpath scanning disabled; Jetty won't discover and run LookupServletContainerInitializer which in turn won't create Lookup and won't set it to the servlet context.

More information at https://mvysny.github.io/npe-staticfilehandler-servestaticresources/

Expected behavior

Since VaadinServlet can not function properly without Lookup, VaadinServlet should throw an IllegalStateException or ServletException that "Vaadin wasn't initialized properly, Lookup is missing in ServletContext, probably LookupServletContainerInitializer wasn't called". Alternatively, a huge error message should be logged in the log, but the exception is far preferable over this.

Minimal reproducible example

#15991 (comment) lists steps to reproduce the issue.

Versions

  • Vaadin / Flow version: Vaadin 23, Vaadin 24, all versions
  • Application Server (if applicable): embedded Jetty without classpath scanning enabled
@mcollovati
Copy link
Collaborator

For the record, this is the change that introduced the check on Lookup presence: #9500.

If I get it correctly, it does not throw because on OSGi the Servlet init method can be called multiple times (#9500 (comment))

@mvysny
Copy link
Member Author

mvysny commented Nov 14, 2024

I suspected that the current bailout was in place because of OSGi. Perhaps, if the servlet can differentiate between an OSGi and non-OSGi environment, we can:

  • Keep the current behavior in the OSGi environment
  • Throw an exception in a non-OSGi environment.

In either case, the return statement would benefit from additional documentation.

@mshabarov
Copy link
Contributor

Throwing an exception look fine, it should suggest how to reconfigure Jetty to be compatible with Vaadin.
By the way, what would be the recommendation? To enable the classpath scanning ?

@mvysny
Copy link
Member Author

mvysny commented Nov 19, 2024

I think enabling the classpath scanning is the easiest way; and that's how servlet containers behave by default when you drop a WAR file in. Alternative would be to provide Jetty with a complete list of all @WebListeners, Servlets and other things; but that's a lot of work, the list can potentially change and therefore this approach is a bit fragile.

This is how Vaadin Boot enables classpath scanning in Jetty: https://github.com/mvysny/vaadin-boot/blob/1bac033680358f363ee8c9bcefe0e0ab175241e6/vaadin-boot/src/main/java/com/github/mvysny/vaadinboot/common/JettyWebServer.java#L156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants