Skip to content

PreMarching filter errors should be filtered as ordinary request filters errors #11723

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

Open
wants to merge 4 commits into
base: 4.8.x
Choose a base branch
from

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
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.

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
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants