Skip to content

Simplify debugging of Undertow transferTo issue #25310 #27908

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

Closed
wants to merge 1 commit into from

Conversation

JiangTChen
Copy link
Contributor

Since undertowHttpServer conflicts with other HTTPServers. Temporarily bypass with transferToForUndertow. It's better than return and pass.

@pivotal-cla
Copy link

@JiangTChen Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 9, 2022
@pivotal-cla
Copy link

@JiangTChen Thank you for signing the Contributor License Agreement!

@sbrannen
Copy link
Member

sbrannen commented Jan 9, 2022

Hi @JiangTChen,

Thanks for submitting your first PR to the Spring Framework!

Since undertowHttpServer conflicts with other HTTPServers. Temporarily bypass with transferToForUndertow.

Can you briefly expound on the first statement and explain how your workaround (the second statement) solves the problem?

@sbrannen sbrannen added in: web Issues in web modules (web, webmvc, webflux, websocket) status: waiting-for-feedback We need additional information before we can continue type: task A general task labels Jan 9, 2022
@sbrannen
Copy link
Member

sbrannen commented Jan 9, 2022

Please note that your PR fails the build:

MultipartIntegrationTests > transferToWithUndertow() FAILED
    java.lang.AssertionError at MessageFormatter.java:115

In light of that, I'm marking this as a draft.

@sbrannen sbrannen marked this pull request as draft January 9, 2022 16:09
@JiangTChen
Copy link
Contributor Author

JiangTChen commented Jan 10, 2022

@sbrannen Thanks for your review.
I found that the pass rate was not 100%. You can use @RepeatedTest(100) instead of @test to verify it.
There are three kinds of exceptions. I think the root cause is about "Connection prematurely closed BEFORE response".
It's very tricky, I'm not sure if it's a configuration of AbstractHttpHandlerIntegrationTests#startServer
or undertow bug.
I will keep digging.
Should we add @Disable on transferToWithUndertow() first, to instead of return pass.
It can alter others and help to fix.

Thanks

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 10, 2022
@JiangTChen JiangTChen marked this pull request as ready for review January 10, 2022 09:33
@sbrannen
Copy link
Member

I found that the pass rate was not 100%. You can use @RepeatedTest(100) instead of @test to verify it. There are three kinds of exceptions. I think the root cause is about "Connection prematurely closed BEFORE response". It's very tricky, I'm not sure if it's a configuration of AbstractHttpHandlerIntegrationTests#startServer or undertow bug. I will keep digging.

Thanks for the explanation.

Should we add @Disable on transferToWithUndertow() first, to instead of return pass. It can alter others and help to fix.

I originally thought you were proposing a fix for Undertow, but since that's not the case, yes adding @Disabled to the new test method is a must.

And please do continue investigating the root cause of the Undertow issue. We haven't had the time to do that ourselves, and we'd be grateful for a fix from the community.

@sbrannen sbrannen changed the title Test transferTo With Undertow httpServer Test transferTo with Undertow in a separate method Jan 10, 2022
@JiangTChen JiangTChen closed this Jan 10, 2022
@JiangTChen JiangTChen reopened this Jan 10, 2022
@JiangTChen JiangTChen changed the title Test transferTo with Undertow in a separate method Assume the unstable undertow transferTo test Jan 10, 2022
@JiangTChen JiangTChen requested a review from sbrannen January 10, 2022 13:33
@JiangTChen
Copy link
Contributor Author

Hi @sbrannen,
Please help to review again.
Thanks.

@sbrannen sbrannen marked this pull request as draft January 13, 2022 16:59
@JiangTChen JiangTChen reopened this Jan 14, 2022
@JiangTChen JiangTChen requested a review from sbrannen January 14, 2022 12:32
@sbrannen sbrannen changed the title Assume the unstable undertow transferTo test Simplify debugging of Undertow transferTo issue #25310 Jan 14, 2022
@sbrannen sbrannen marked this pull request as ready for review January 14, 2022 14:42
@sbrannen sbrannen self-assigned this Jan 14, 2022
@sbrannen sbrannen removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jan 14, 2022
@sbrannen sbrannen added this to the 6.0.0-M3 milestone Jan 14, 2022
@sbrannen sbrannen closed this in f43fb41 Jan 14, 2022
sbrannen added a commit that referenced this pull request Jan 14, 2022
@sbrannen
Copy link
Member

This has been merged into main.

Feel free to continue to investigate the cause of the issue and report your findings in #25310.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants