Delegate HTTP status resolution to PolarisException hierarchy

[harshitaajoshi]

Summary

Today PolarisExceptionMapper maintains a centralized switch statement that maps every PolarisException subtype to an HTTP status code. This creates tight coupling between the mapper and every exception class, and requires updating the mapper whenever a new exception type is added.

This PR inverts that relationship by adding an abstract httpStatusCode() method to PolarisException and implementing it in each subclass with the correct status code. The mapper now delegates to exception.httpStatusCode() instead of maintaining the switch.

The method returns a plain int so the signature stays transport neutral. The implementations reference Response.Status constants, which is fine because jakarta.ws.rs is already a declared dependency of polaris-core. Every subclass now declares its own status explicitly, including FileIOUnknownHostException, which returns 500 to match its current behavior.

Fixes #5167

Checklist

• Don't disclose security issues! (contact security@apache.org)
• Clearly explained why the changes are needed, or linked related issues: Fixes Refactor PolarisException status mapping into PolarisException hierarchy #5167
• Added/updated tests with good coverage, or manually tested (and explained how)
• Added comments for complex logic
• Updated CHANGELOG.md (if needed)
• Updated documentation in site/content/in-dev/unreleased (if needed)

Test plan

• Added a parameterized test (testPolarisExceptionStatusCodes) covering all 10 PolarisException subclasses, verifying both httpStatusCode() on the exception and the HTTP response from the mapper.
• Existing ExceptionMapperTest cases continue to pass unchanged.
• Ran ./gradlew format compileAll and ./gradlew :polaris-core:check locally with all tests passing.

[flyrain]

Thanks @harshitaajoshi for the PR. The refactor seems straightforward. I'm OK with the direction. One minor concern is that, it pushes an HTTP/transport concept (status codes) into polaris-core, which is usually transport-neutral. I'd suggest a dev emailing discussion to gather more inputs before we merging it.

[harshitaajoshi]

Thanks for the review and the approval, @flyrain. That is a fair point about keeping polaris-core transport-neutral. I have started a thread on the dev mailing list to gather broader input before merging. Happy to adjust the approach if the community prefers keeping the status mapping entirely in the service layer.

[vigneshio]

Thanks @harshitaajoshi - LGTM... +1 with @flyrain

[adutra]

Note: even if I agree with this method returning raw ints, the Jakarta Jax-RS API is a direct dependency of polaris-core:

polaris/polaris-core/build.gradle.kts

Line 58 in 79cabc5

implementation(libs.jakarta.ws.rs.api)

It is therefore possible to replace the int constants with a call to that API, as follows:

public int httpStatusCode() {
  return Response.Status.INTERNAL_SERVER_ERROR.getStatusCode();
}

[harshitaajoshi]

Thanks for the review and the context, @adutra. Good to know that jakarta.ws.rs is already available in polaris-core. I went with raw ints to keep the exception classes minimal, but I can switch to the Response.Status constants if you prefer. Let me know.

[adutra]

Not that I have a strong preference, but let's use Response.Status constants. I hope everybody will agree.

[harshitaajoshi]

Done, switched to Response.Status constants in the latest commit. I kept the literal values in the test assertions so the test does not depend on the same constants as the production code.

[adutra]

Thanks! I think that this change did improve readability a bit.

[flyingImer]

Like the direction here. The mapper doesn't need to know about every PolarisException subtype anymore, and that drops eight cross-module imports.

One thing worth coordinating on: 4961 is also open and mergeable, and it adds two new PolarisException subclasses through the same getStatus switch this PR deletes. Whoever lands second will hit a conflict, and the fix that compiles is just dropping those two case arms, not adding httpStatusCode overrides for them. Nothing forces that second step, so both could quietly end up at 500. Left a separate comment on the httpStatusCode default itself for the related point.

One smaller thing: the description still says this keeps jakarta.ws out of polaris-core, but the touched classes now import jakarta.ws.rs.core.Response. I'd rather core stay framework-free in source since nothing else in it uses jakarta today, but the dependency was already declared so this isn't new exposure, and I'm not blocking on it.

[flyingImer]

The referenced pr 5167 asked for a required method here, and this ships a default returning 500. FileIOUnknownHostException shows why that gap matters: it's the one subclass with no override, the new test pins it at 500, and IcebergExceptionMapper maps the same class to 404 elsewhere in the tree.

I'd just drop the default and make this abstract, since every subclass but that one already overrides it, it'll compile everywhere else and force a real answer here instead of an inherited one. No source outside this repo extends PolarisException today, so I don't think it costs any real compatibility.

[harshitaajoshi]

Good point, done.

Made httpStatusCode() abstract and gave FileIOUnknownHostException an explicit 500. That is its current runtime behavior, since PolarisExceptionMapper is the more specific mapper and wins for that class, so this stays a pure refactor. It also matches what #5167 originally asked for.

I did not align it with the 404 in IcebergExceptionMapper because that is a real behavior change and deserves its own issue rather than riding along here.

@adutra @flyrain heads up, this reverses the non abstract default you both approved earlier. The reasoning is that FileIOUnknownHostException was the only subclass leaning on the inherited value and it was silently disagreeing with IcebergExceptionMapper. Easy to revert if you would rather keep the default, just say the word.

On #4961: agreed, whoever lands second should add overrides for the two new subclasses rather than dropping the case arms. Happy to rebase if it lands first.

Description updated.