TOMEE-4642 - Don't fail deployment when a servlet/filter/listener class is missing

[jungm]

What

When a war's web.xml or annotations name a servlet, filter or listener class that is not packaged in the war, AnnotationDeployer.ProcessAnnotatedBeans.deploy(WebModule) rethrew the ClassNotFoundException/NoClassDefFoundError as an OpenEJBException. That propagates out through ConfigurationFactory.configureApplication and aborts startup of the whole web context.

This changes the servlet, filter and listener paths to log a warning and continue instead of failing.

Why

These classLoader.loadClass(...) calls exist only to feed the annotation scanner. A class that cannot be loaded simply contributes nothing to scan — it should not bring the context down.

The old behaviour was also internally inconsistent: the taglib-listener loop right below, and the servlet name-fallback case, already only logged and continued. Only the explicit-class servlet/filter/listener paths were fatal.

Jakarta Servlet 6.1 §2.3.1 ("Loading and Instantiation") permits servlet loading to be "delayed until the container determines the servlet is needed to service a request", so deferring an unresolved class rather than failing eagerly at deploy time is spec-compliant. If such a component is actually used, Tomcat still surfaces the missing class per-component.

The WsDeployer throw for webservice servlet classes was intentionally left as-is, since a WS endpoint genuinely cannot be built without its class.

Also

Fixed off-by-one MessageFormat placeholder indices ({1}{2}{3} → {0}{1}{2}) in the four related logger.debug calls, which were dropping the first argument from the message.

Testing

Added AnnotationDeployerTest.missingServletFilterAndListenerClassesDoNotFailDeployment, using the class names from the ticket (TestServlet1, AddFilterString, plus a missing listener). Verified it is a real regression test: reverting the servlet fix makes it fail with exactly the reported error (OpenEJBException: Unable to load servlet class: ...TestServlet1), and it passes with the fix.

This is the TomEE-side fix for the two Jakarta Servlet TCK deployments that triggered the abort (RegistrationTests naming filter AddFilterString; DefaultMappingTests naming servlet TestServlet1). Removing the corresponding entries from runner-standalone/exclusions/servlet.txt in the apache/tomee-tck harness and confirming both classes pass is a follow-up in that separate repo.

Jira: https://issues.apache.org/jira/browse/TOMEE-4642

🤖 Generated with Claude Code

[rzo1]

The direction is right and the codebase already agrees with you —
FinderFactory.createFinder (:235-243) swallows CNFE for these same servlet classes, the
taglib listener loop two blocks below only logs, and the EJB path at :2447 does continue.
The three fatal throws were the odd ones out. The {1}{2}{3} -> {0}{1}{2} MessageFormat
fix is a real bug fix on its own. And the test genuinely regresses: reverting only
AnnotationDeployer.java gives OpenEJBException: Unable to load servlet class: ...TestServlet1.

But the reported bug is not actually fixed on half the distributions:

• ConfigurationFactory:146 defines WSDL4J_AVAILABLE, and :260-262 adds new WsDeployer()
  to the deployer chain whenever openejb.webservices.enabled (default true) &&
  WSDL4J_AVAILABLE. WsDeployer.processPorts(WebModule) iterates every servlet of the
  WebApp — only skipping className == null for JSPs — and does
  webModule.getClassLoader().loadClass(className) at ~:152, inside a try whose catch at
  :233-235 is catch (final Exception e) { throw new OpenEJBException("Unable to load servlet class: " + className, e); }.

  The loadClass precedes the JaxWsUtils.isWebService(clazz) test, so it is not limited
  to WS endpoints. libre-wsdl4j is optional in openejb-core's pom (:612-616) but is present
  on Plus, Plume and openejb-standalone. So on those distributions the war that motivated
  this ticket still fails to deploy, with an identically worded error from a different class.

  Note when you fix it that NoClassDefFoundError is an Error, so catch (Exception)
  there doesn't catch it at all — it propagates raw today.

Consequently the test can't do the job it's meant to do:

• It drives AnnotationDeployer directly, so it would have stayed green through exactly the
  gap above. Something at ConfigurationFactory.configureApplication level, or an Arquillian
  test with a real war, is what would actually pin TOMEE-4642.

Other things:

• The caught Throwable is discarded entirely — logger.warning("Unable to load servlet class: " + servletClass)
  drops both the cause and the module identity. Whoever hits this because they genuinely
  mis-packaged a war now gets one line and no stack trace, on a path that used to be fatal
  and loud. Please pass e and include webModule.getJarLocation().

• The Servlet 6.1 §2.3.1 rationale in the comment covers servlets, whose loading may be
  deferred. It does not cover filters or listeners — the container must instantiate those at
  context start, so a missing class there is unambiguously broken, just broken later. Worth
  either reflecting that in the comments or keeping a stronger log level for those two.

• Relatedly, LightweightWebAppBuilder:183-186 now regresses to a raw ClassNotFoundException
  mid-createApplication instead of the clean OpenEJBException it used to get.

• Fail-open side effect worth acknowledging: an unloadable servlet class now silently skips
  @Resource/@EJB annotation processing for that servlet, so if the class is resolvable
  later by a different loader it comes up without its injections rather than not at all.

• The MessageFormat index fix is applied to 4 of 12 identical occurrences in this file —
  :2019, :2044, :2061, :2192, :2349, :2379, :2929 and :5696 have the same off-by-one. Either
  do them all or split that into its own trivial commit.

• Twenty lines below your last hunk, the webservices handler-chain loop still does
  throw new OpenEJBException("Unable to load webservice handler class: " + handlerClass, e)
  from the identical catch shape. Keeping a declared handler fatal is defensible, but the
  method now has three tolerant sites, two logger.error sites and one fatal site with
  nothing saying why. One comment would do.

[jungm]

Thanks — you were right that the bug wasn't actually fixed. Verified every claim before acting; all confirmed. Pushed 3 commits.

WsDeployer (the real gap). Confirmed: ConfigurationFactory:260-262 adds it whenever openejb.webservices.enabled && WSDL4J_AVAILABLE, libre-wsdl4j is <optional> at pom :612-616, and processPorts does loadClass at :150 before the isWebService check at :151 — so it hits every servlet, not just WS endpoints. Now resolved in its own try/catch that skips webservice detection for that servlet and continues. Also caught NoClassDefFoundError explicitly, since as you noted catch (Exception) never covered it. The remaining catch (Exception) can now only fire on genuine WS configuration failures, so I reworded its message rather than leaving it claiming a load failure.

The test couldn't do its job. Correct — and demonstrated: added missingServletClassDoesNotFailFullConfiguration, which drives ConfigurationFactory.configureApplication so the whole chain runs. Reverting only WsDeployer.java makes it fail with OpenEJBException: Unable to load servlet class: ...TestServlet1, while the AnnotationDeployer warning still logs first — exactly the gap you described. wsdl4j is on the test classpath, so WsDeployer genuinely runs. Kept the original test too, since it pins the AnnotationDeployer behavior directly.

Discarded cause. Fixed — all sites now pass e and webModule.getJarLocation().

Filters/listeners vs §2.3.1. You're right that §2.3.1's deferral allowance covers servlets only. Both now log at error rather than warning. I read the spec source at tag 6.1.0-RELEASE rather than trusting my earlier summary: §6.2.1 says the container instantiates a filter "before a request causes the container to access a web resource" — later than deploy, but not skippable if the filter is ever used — so the comment now says that instead of the "must be instantiated at context start" I'd first written. Worth noting the spec's must fail to deploy clauses (:3880, :3991, :4274, :4364, :4407) are all web-fragment ordering conflicts, not unresolvable component classes, so nothing there requires the old fatal behavior.

LightweightWebAppBuilder:183-186. Confirmed and fixed — it now skips the unloadable listener and logs at error, instead of throwing a raw CNFE mid-createApplication.

Fail-open side effect. Agreed; called it out explicitly in the servlet comment.

MessageFormat. Did all of them — remaining 7 in a separate commit (a6e030785e), message text untouched, indices only.

Handler chain. Left fatal, now with a comment saying why: a silently missing handler leaves the endpoint running under a weaker contract than declared (e.g. a dropped security handler), which is worse than disabling one component.

One thing I did not do: removing the runner-standalone/exclusions/servlet.txt entries and confirming both TCK classes pass, since that's the separate apache/tomee-tck repo and needs a full run against a built server.

Unrelated pre-existing flakiness worth flagging: WebAppEnvEntryTest, AutoDeployerTest, BuiltInEnvironmentEntriesTest, VagueEnvEntriesMatchTest and ActivationConfigPropertyOverrideTest fail non-deterministically from shared static state — two identical runs of the same code give different failure sets, and they fail on unmodified main too.

🤖 Addressed by Claude Code