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

Don't write error response when it's already been written #17551

Merged
merged 1 commit into from
May 31, 2021

Conversation

geoand
Copy link
Contributor

@geoand geoand commented May 31, 2021

The original failure that Vertx had captured and was trying to be handled by QuarkusErrorHandler was io.netty.channel.StacklessClosedChannelException

Relates to: #17548

@sberyozkin
Copy link
Member

Thanks Georgios for a super fast fix, which is also simple, hence even I can approve it :-)

@gsmet
Copy link
Member

gsmet commented May 31, 2021

It might be a dumb question but why do we have an error in the first place given the OP is just clicking around?

@sberyozkin sberyozkin self-requested a review May 31, 2021 11:28
@sberyozkin
Copy link
Member

@gsmet, Hi Guillaume, the devconsole handlers are implemented as Vert.x handlers, it may explain it

@sberyozkin
Copy link
Member

@geoand
Copy link
Contributor Author

geoand commented May 31, 2021

aroun

It seems like Netty might close a Channel and cause this.
I don't know if that is new to the latest Netty version or not

@gsmet
Copy link
Member

gsmet commented May 31, 2021

Yeah. For me, that looks very very suspicious. I wouldn't expect Netty to throw random errors after a few simple GETs.

@geoand
Copy link
Contributor Author

geoand commented May 31, 2021

That may or may not be true to be honest. I'll look a little more, but I doubt there is an actual problem here and even if there is, there is likely very little we can do about it

@sberyozkin
Copy link
Member

We already had something similar fixed before: https://github.com/quarkusio/quarkus/pull/13060/files

@gsmet
Copy link
Member

gsmet commented May 31, 2021

Well, I'm not saying it would be our own bug to fix, it might be an issue elsewhere. But I fail to see how this could be a normal behavior.

Let's not merge this too fast and wait for @stuartwdouglas to have a look if he can.

@geoand
Copy link
Contributor Author

geoand commented May 31, 2021

It looks like the StacklessClosedChannelException exception is being thrown by io.vertx.core.http.impl.WebSocketImplBase#closeConnection, I guess this is for @pmlopes or @vietj to take a look

@geoand
Copy link
Contributor Author

geoand commented May 31, 2021

Well, I'm not saying it would be our own bug to fix, it might be an issue elsewhere. But I fail to see how this could be a normal behavior.

Let's not merge this too fast and wait for @stuartwdouglas to have a look if he can.

See my latest comment.

It's only happens in web sockets (which the DevUI uses) and in any case this fix should be in place anyway

@sberyozkin
Copy link
Member

sberyozkin commented May 31, 2021

It's only happens in web sockets (which the DevUI uses) and in any case this fix should be in place anyway

Makes sense, perhaps removing Fixes #17548 would be a good compromise for the underlying cause be investigated as part of #17548 ? preventing the confusing error message in the logs is indeed needed in any case

@geoand
Copy link
Contributor Author

geoand commented May 31, 2021

Sure yeah, done :)

@sberyozkin
Copy link
Member

Look forward to rebasing my Keycloak Devservices PR :-),
I suppose we can merge it now, Guillaume, hope you are OK with it, I've added a note to ##17548 that you'd like the lower level issue investigated, Stuart and others may have some ideas

@sberyozkin sberyozkin merged commit d42d422 into quarkusio:main May 31, 2021
@quarkus-bot quarkus-bot bot added this to the 2.1 - main milestone May 31, 2021
@geoand geoand deleted the #17548 branch May 31, 2021 16:20
@gsmet gsmet modified the milestones: 2.1 - main, 2.0.0.Final May 31, 2021
@doublefx
Copy link

doublefx commented Aug 2, 2021

@sberyozkin

Same issue just happened again using Quarkus 2.1.0-FINAL, opening Swagger

2-Aug 14:04:24.097 INFO  [io.ve.ex.we.RoutingContext] (vert.x-eventloop-thread-7) RoutingContext failure (500): io.netty.channel.StacklessClosedChannelException
	at io.netty.channel.AbstractChannel$AbstractUnsafe.write(Object, ChannelPromise)(Unknown Source)

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

Successfully merging this pull request may close these issues.

None yet

5 participants