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

Give better errors for non public Websocket Endpoints #4903

Closed
fredbar opened this issue May 22, 2020 · 5 comments · Fixed by #4906 or #4959
Closed

Give better errors for non public Websocket Endpoints #4903

fredbar opened this issue May 22, 2020 · 5 comments · Fixed by #4906 or #4959

Comments

@fredbar
Copy link

fredbar commented May 22, 2020

based on org.eclipse.jetty.websocket:javax-websocket-server-impl:jar:9.4.28.v20200408

If an Endpoint is a private inner class, jetty fails silently, responding with a 503. This is due to the fact that the ContainerDefaultConfigurator looks for a static initializer and the fact that inner classes can not have them.

Following is a stack trace for the call, gotten when enabling debug log traces.

java.lang.InstantiationException: java.lang.NoSuchMethodException: com.xxx$MyServer.<init>()
	at org.eclipse.jetty.websocket.jsr356.server.ContainerDefaultConfigurator.getEndpointInstance(ContainerDefaultConfigurator.java:75)
	at org.eclipse.jetty.websocket.jsr356.server.JsrCreator.createWebSocket(JsrCreator.java:155)
	at org.eclipse.jetty.websocket.server.WebSocketServerFactory.acceptWebSocket(WebSocketServerFactory.java:230)
	at org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter.doFilter(WebSocketUpgradeFilter.java:258)
	at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1618)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:549)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1610)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1363)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:489)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1580)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1278)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
	at org.eclipse.jetty.server.Server.handle(Server.java:500)
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:383)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:547)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:375)
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:273)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
	at org.eclipse.jetty.io.ssl.SslConnection$DecryptedEndPoint.onFillable(SslConnection.java:543)
	at org.eclipse.jetty.io.ssl.SslConnection.onFillable(SslConnection.java:398)
	at org.eclipse.jetty.io.ssl.SslConnection$2.succeeded(SslConnection.java:161)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
	at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:117)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:336)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:313)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:171)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:129)
	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:375)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)
	at java.base/java.lang.Thread.run(Thread.java:834)

There are two points, i might have incorrect assumptions but here they are:

  • i would have expected the exception to be thrown or at least printed so that problem can be found rapidly
  • i would have expected a type 500 error for such a problem, which would have put me in the right direction much faster.
@lachlan-roberts
Copy link
Contributor

@fredbar You must declare your inner class as public static for this to work.
But I agree that we could do a better job at reporting these errors. I think in this case jetty should throw DeploymentException when the endpoint is added to the ServerContainer, this way you get an error at deployment time and not just when the first request comes in.

lachlan-roberts added a commit that referenced this issue May 25, 2020
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue May 25, 2020
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@fredbar
Copy link
Author

fredbar commented May 25, 2020

Oh, sure, having a deployment exception is even better.
Thanks for your work on this.

@gregw
Copy link
Contributor

gregw commented May 25, 2020

@lachlan-roberts Also that thrown exception is losing the root cause. It does echo the class and message, but the stack is lost. Can we set the cause on that exception as well.

lachlan-roberts added a commit that referenced this issue May 25, 2020
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue May 26, 2020
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue May 26, 2020
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue May 26, 2020
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue May 26, 2020
…cModifier

Issue #4903 - give better errors for non public javax.websocket endpoints
@lachlan-roberts
Copy link
Contributor

Fixed with PR #4906, will be released with 9.4.30.

lachlan-roberts added a commit that referenced this issue May 28, 2020
…ints jetty-10

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue May 29, 2020
…ateEndpoints

Issue #4903 - give better errors for non public javax.websocket endpoints (jetty-10)
@joakime joakime changed the title websocket endpoints can not be inner classes Websocket Endpoints can not be inner classes Jun 10, 2020
@joakime joakime changed the title Websocket Endpoints can not be inner classes Give better errors for non public Websocket Endpoints Jun 10, 2020
@joakime joakime reopened this Jun 10, 2020
@joakime
Copy link
Contributor

joakime commented Jun 10, 2020

This change has broken endpoints that extend from javax.websocket.Endpoint and do not have public default constructors. (not a requirement of the spec)

joakime added a commit that referenced this issue Jun 10, 2020
…gurator

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Jun 10, 2020
…gurator

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
lachlan-roberts added a commit that referenced this issue Jun 11, 2020
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Jun 11, 2020
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
joakime added a commit that referenced this issue Jun 11, 2020
…point-check

Issue #4903 - Improved behavior for Custom ServerEndpointConfig.Configurator
@joakime joakime closed this as completed Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants