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

There are some problems with the response to the client request for 304 #2818

Closed
poor-girl opened this issue Jun 1, 2023 · 4 comments · Fixed by #2820
Closed

There are some problems with the response to the client request for 304 #2818

poor-girl opened this issue Jun 1, 2023 · 4 comments · Fixed by #2820
Assignees
Labels
type/bug A general bug
Milestone

Comments

@poor-girl
Copy link

After we upgraded the version of reactor-netty-http from 1.0.26 to 1.0.27, we found that some clients had the 'unexpected end of stream' issue for 304 response requests. When we verified, we found that the lower version of postman would have the problem that the request connection was not disconnected.

Expected Behavior

The request responds normally to 304

Actual Behavior

some clients had the 'unexpected end of stream' issue.
With version 8.0.7 of Postman will have a 304 already responding, but the connection is not disconnected

Steps to Reproduce

    public Mono<ConfigsResponseBody> pollConfigsRelease(String instanceKey) {
        return Mono.<ConfigsResponseBody>create(request -> {
            // ConcurrentMap<String, MonoSink<ConfigsResponseBody>> requestPool
            requestPool.put(instanceKey, request);
            executorService.execute(this::handlerFullConfig);

        })
            .timeout(Duration.ofSeconds(5L),
                Mono.defer(() -> Mono.error(new ResponseStatusException(HttpStatus.NOT_MODIFIED))))
            .doFinally(signalType -> {
                requestPool.remove(instanceKey);
                log.info("request from {} is return, signalType is {}", instanceKey, signalType);
            });
    }

Possible Solution

I found a patch that could cause this issue:
fcda69a?diff=unified

Your Environment

  • Reactor version(s) used:reactor-netty-http 1.0.27+
  • Other relevant libraries versions (eg. netty, ...):
  • JVM version (java -version): 8
  • OS and version (eg. uname -a): Ubuntu4.15.0-91-generic x86_64 x86_64 x86_64 GNU/Linux or window10
@poor-girl poor-girl added status/need-triage A new issue that still need to be evaluated as a whole type/bug A general bug labels Jun 1, 2023
@poor-girl
Copy link
Author

image

@violetagg violetagg removed the status/need-triage A new issue that still need to be evaluated as a whole label Jun 1, 2023
@violetagg violetagg self-assigned this Jun 1, 2023
@violetagg
Copy link
Member

@poor-girl Please provide reproducible example. The snippet above is not enough.

@violetagg violetagg added the for/user-attention This issue needs user attention (feedback, rework, etc...) label Jun 1, 2023
@poor-girl
Copy link
Author

Create a spring project,ensure reactor-netty-http version 1.0.27+

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>
    <parent>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-parent</artifactId>
        <version>2.7.9</version>
        <relativePath/> <!-- ensure reactor-netty-http version 1.0.27+, this is 1.0.28 -->
    </parent>
    <groupId>com.example</groupId>
    <artifactId>demo-flux-web</artifactId>
    <version>0.0.1-SNAPSHOT</version>
    <name>demo-flux-web</name>
    <description>Demo project for Spring Boot</description>
    <properties>
        <java.version>8</java.version>
    </properties>
    <dependencies>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter</artifactId>
        </dependency>

        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-webflux</artifactId>
        </dependency>

        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-test</artifactId>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>io.swagger</groupId>
            <artifactId>swagger-annotations</artifactId>
            <version>1.6.9</version>
        </dependency>
        <dependency>
            <groupId>jakarta.validation</groupId>
            <artifactId>jakarta.validation-api</artifactId>
        </dependency>
        <dependency>
            <groupId>org.projectlombok</groupId>
            <artifactId>lombok</artifactId>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>org.springframework.boot</groupId>
                <artifactId>spring-boot-maven-plugin</artifactId>
            </plugin>
        </plugins>
    </build>

</project>

@Api(value = "TestController")
public interface TestControllerApi {

    @ApiResponses(value = {
        @ApiResponse(code = 304, message = "no release"), @ApiResponse(code = 404, message = "no full configs")
    })
    @GetMapping(value = "/v1/configs/test", produces = {"application/json"})
    default Mono<String> test() {
        throw new ResponseStatusException(HttpStatus.NOT_IMPLEMENTED);
    }

}
@RestController
@Slf4j
public class TestController implements TestControllerApi {

    private final ConcurrentMap<String, MonoSink<String>> requestPool = new ConcurrentHashMap<>();

    @Override
    public Mono<String> test() {
        return pollConfigsRelease("test");
    }

    public Mono<String> pollConfigsRelease(String instanceKey) {
        return Mono.<String>create(request -> {
            requestPool.put(instanceKey, request);
            // do something
        })
            .timeout(Duration.ofSeconds(5L),
                Mono.defer(() -> Mono.error(new ResponseStatusException(HttpStatus.NOT_MODIFIED))))
            .doFinally(signalType -> {
                requestPool.remove(instanceKey);
                log.info("request from {} is return, signalType is {}", instanceKey, signalType);
            });
    }
}

Start the application,use postman version 8.0.7 to reproduce

image

@violetagg violetagg removed the for/user-attention This issue needs user attention (feedback, rework, etc...) label Jun 2, 2023
@violetagg violetagg added this to the 1.0.33 milestone Jun 2, 2023
@violetagg
Copy link
Member

@poor-girl If you can test the 1.0.33-SNAPSHOT it will be great! Thanks for the reproducible example!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants