Skip to content

Conversation

@dstepanov
Copy link
Contributor

@dstepanov dstepanov commented Apr 7, 2025

When error occurs the exeption mapper response should be processed by the response filters

@dstepanov dstepanov added the type: bug Something isn't working label Apr 7, 2025
@dstepanov dstepanov requested review from graemerocher and yawkat April 7, 2025 12:32
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 7, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 New Bugs (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@yawkat
Copy link
Member

yawkat commented Apr 14, 2025

This does not solve the issue I mentioned. With this change, normal response filters still run when a PreMatching request filter returns an early response, which is undesirable.

Response filters should run if and only if a request filter of the same priority also runs.

@dstepanov
Copy link
Contributor Author

I would say it’s desirable considering JaxRs is working in the same way, if the request pre matching commits a response the response filters run, the behavior matches the ordinary request filters.

@yawkat
Copy link
Member

yawkat commented Apr 14, 2025

No, normal filters work like I describe.

@RequestFilter("/foo")
@Order(0)
HttpResponse<String> requestFilterA() {
    System.out.println("RequestFilterA");
    return HttpResponse.ok("foo");
}

@ResponseFilter("/foo")
@Order(-1)
void responseFilterB() {
    System.out.println("ResponseFilterB");
}

@ResponseFilter("/foo")
@Order(1)
void responseFilterC() {
    System.out.println("ResponseFilterC");
}

This will run filters A and B but not filter C.

With your change, if there is a PreRouting filter, B and C can run without A running at all.

@dstepanov
Copy link
Contributor Author

No it's not, in your example the prerouting filters work the same way, filter C will not run.

@dstepanov dstepanov changed the base branch from 4.8.x to 4.9.x May 26, 2025 12:18
@dstepanov dstepanov closed this May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something isn't working

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants