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

Prevent InvalidStateException when response has already ended. #951

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Prevent InvalidStateException when response has already ended. #951

wants to merge 4 commits into from

Conversation

feg624
Copy link

@feg624 feg624 commented Jun 18, 2018

This PR prevents the scenario described on the stacktrace below. It happens when a prior handler than the template handler, breaks the handler chain while performing some validation. This change first checks whether the response has already ended before trying to write the template result to the response.

Jun 18, 2018 8:34:57 PM io.vertx.ext.web.impl.RoutingContextImplBase
SEVERE: Unexpected exception in route
java.lang.IllegalStateException: Response has already been written
at io.vertx.core.http.impl.HttpServerResponseImpl.checkValid(HttpServerResponseImpl.java:545)
at io.vertx.core.http.impl.HttpServerResponseImpl.putHeader(HttpServerResponseImpl.java:189)
at io.vertx.ext.web.handler.impl.TemplateHandlerImpl.lambda$handle$0(TemplateHandlerImpl.java:50)
at io.vertx.ext.web.templ.impl.PebbleTemplateEngineImpl.render(PebbleTemplateEngineImpl.java:89)
at io.vertx.ext.web.handler.impl.TemplateHandlerImpl.handle(TemplateHandlerImpl.java:48)
at io.vertx.ext.web.handler.impl.TemplateHandlerImpl.handle(TemplateHandlerImpl.java:28)
at io.vertx.ext.web.impl.RouteImpl.handleContext(RouteImpl.java:219)
at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:120)
at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:133)
at com.fegsoft.pricinglist.AppServerVerticle.lambda$5(AppServerVerticle.java:128)
at io.vertx.core.eventbus.impl.EventBusImpl.lambda$convertHandler$1(EventBusImpl.java:349)
at io.vertx.core.eventbus.impl.HandlerRegistration.deliver(HandlerRegistration.java:223)
at io.vertx.core.eventbus.impl.HandlerRegistration.handle(HandlerRegistration.java:200)
at io.vertx.core.eventbus.impl.EventBusImpl.lambda$deliverToHandler$3(EventBusImpl.java:533)
at io.vertx.core.impl.ContextImpl.lambda$wrapTask$2(ContextImpl.java:339)
at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:163)
at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:404)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:463)
at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:886)
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.lang.Thread.run(Thread.java:745)

@feg624
Copy link
Author

feg624 commented Jun 19, 2018

I've added more changes to the PR, so that the RoutingContext is available after reroute. These changes may not be good enough, but allows to start discussions about it.

@feg624 feg624 closed this Jun 19, 2018
@pmlopes pmlopes removed the to review label Jun 19, 2018
@feg624 feg624 reopened this Jun 19, 2018
@feg624
Copy link
Author

feg624 commented Jun 19, 2018

I reverted the last commit, as I realized it was working as designed. However, I still think that the TemplateHandlerImpl should check whether the response has already ended.

@pmlopes
Copy link
Member

pmlopes commented Jun 27, 2019

I think this fix is applied in the wrong place. Any handler can break the chain so this issue affects not just the template engine but any handler. I think the check should be done in the iteration.

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

Successfully merging this pull request may close these issues.

3 participants