Generate sealed interfaces to match against endpoint associated errors by mpritham · Pull Request #2696 · palantir/conjure-java

mpritham · 2025-10-27T22:24:45Z

Before this PR

In #2401 we added support for defining endpoint associated errors. This allowed Conjure service owners to define the errors that clients should expect to handle. This work was extended in #2435 where Dialogue client interfaces were generated to return sealed interfaces permitting records that corresponded to each endpoint error, and a "success" type, acting as a "result" type.

Initially the stance was that endpoint errors defined by services need to be handled by clients: using result types (or checked exceptions) would nudge callers of a client's endpoint to handle all of the potential errors a service has associated with the endpoint. (This could be worked around by utility methods that extract specific "success" types out of the provided result type but that would be an intentional choice made by clients.) In practice, this is a restrictive stance that hurts adoption: client code does not always need to handle the errors that endpoints have associated but if they would like to, they should have a way to know what errors should be handled.

Another reason to not change the return type of all endpoints in all Dialogue client interfaces is that it could lead to ABI breaks.

After this PR

For each endpoint that has associated errors in its definition, there will be a new sealed interface generated that allows devs to convert a generic RemoteException into one of the error-specific exceptions. The sealed interface allows devs on Java 21+ to use exhaustive pattern matching switch statements providing compile-time guarantees that they have handled all possible endpoint associated errors.

For example, consider this endpoint definition:

ErrorService:
  name: Error Service
  package: com.palantir.product
  default-auth: header
  base-path: /errors
  endpoints:
    testBasicError:
      http: POST /basic
      args:
        shouldThrowError: boolean
      returns: string
      errors:
        - InvalidArgument
        - ConflictingCauseSafeArgErr

The following Dialogue client interface will be generated:

public interface ErrorServiceBlocking {
    /** @apiNote {@code POST /errors/basic} */
    @ClientEndpoint(method = "POST", path = "/errors/basic")
    String testBasicError(AuthHeader authHeader, boolean shouldThrowError);
    
    // <various other methods not relevant to this feature>
    
    sealed interface TestBasicErrorErrors {
        static TestBasicErrorErrors from(RemoteException e) {
            if (e instanceof TestErrors.InvalidArgumentException ex) {
                return new InvalidArgument(ex);
            } else if (e instanceof ConjureErrors.ConflictingCauseSafeArgErrException ex) {
                return new ConflictingCauseSafeArgErr(ex);
            } else {
                return new Unknown(e);
            }
        }

        record InvalidArgument(TestErrors.InvalidArgumentException e) implements TestBasicErrorErrors {}

        record ConflictingCauseSafeArgErr(ConjureErrors.ConflictingCauseSafeArgErrException e)
                implements TestBasicErrorErrors {}

        record Unknown(RemoteException e) implements TestBasicErrorErrors {}
    }
}

This allows clients to catch RemoteExceptions thrown by the call to testBasicError and convert it into a TestBasicErrorErrors object.

==COMMIT_MSG==
Generate sealed interfaces to match against endpoint associated errors
==COMMIT_MSG==

Possible downsides?

changelog-app · 2025-10-27T22:24:49Z

Generate changelog in `changelog/@unreleased`

Type (Select exactly one)

Feature (Adding new functionality)
Improvement (Improving existing functionality)
Fix (Fixing an issue with existing functionality)
Break (Creating a new major version by breaking public APIs)
Deprecation (Removing functionality in a non-breaking way)
Migration (Automatically moving data/functionality to a new system)

Description

Generate sealed interfaces to match against endpoint associated errors

Check the box to generate changelog(s)

Generate changelog entry

mpritham · 2025-10-27T22:25:52Z

...core/src/integrationInput/java/endpointerrors/com/palantir/product/ErrorServiceBlocking.java

+        record Unknown(RemoteException e) implements TestBasicErrorErrors {}
+    }
+
+    sealed interface TestImportedErrorErrors {


Probably nicer for the suffix to be singular.

For reviewers: this file (or the async interface) contains the relevant newly generated code.

Singular definitely makes sense since the actual instance will be a single type. Also thoughts on ErrorType or something for the suffix? Was also thinking AssociatedError, but that feels like a mouthful

hm, what are your thoughts on just keeping the name <EndpointName>Error? Feel like it's cleaner.

sure works for me

mpritham · 2025-10-27T22:30:00Z

...re/src/main/java/com/palantir/conjure/java/services/dialogue/DialogueInterfaceGenerator.java

+                    ErrorGenerationUtils.errorTypesClassName(error.getError().getNamespace()),
+                    ErrorGenerationUtils.errorExceptionClassName(errorName));
+            TypeSpec errorRecord = createRecordForEndpointErrorUtility(errorName, exceptionClassName, utiltyClassName);
+            // Not strictly required. Seems a bit cleaner to not have it.


This is because of JLS-9.1.4. I'll remove the comment.

I suspect these sealed interfaces are small so it might be quite easy to just see all of the sub-interfaces. For things like the generated union classes (which were much larger), I think it's nice to have the list of permitted interfaces in one line at a glance. I can be convinced that these sealed interfaces could be large for endpoints with loads of associated errors and we should add them. Not super opinionated here.

mpritham · 2025-10-28T00:50:55Z

...core/src/integrationInput/java/endpointerrors/com/palantir/product/ErrorServiceBlocking.java

+
+        record EndpointError(EndpointSpecificErrors.EndpointErrorException e) implements TestImportedErrorErrors {}
+
+        record Unknown(RemoteException e) implements TestImportedErrorErrors {}


I initially did not have this Unknown type, and just threw an exception if the RemoteException wasn't an endpoint associated error, but I think this will lead to nested try-catch blocks in code. Devs will likely want to handle the "unknown" case anyway so creating a separate record for it that they can match on makes sense.

mpritham · 2025-10-28T14:53:43Z

...java-core/src/test/java/com/palantir/conjure/java/endpointerrors/UndertowServiceEteTest.java

+        } catch (RemoteException e) {
+            TestBasicErrorErrors error = ErrorServiceBlocking.TestBasicErrorErrors.from(e);
+            assertThat(error).isInstanceOfSatisfying(TestBasicErrorErrors.InvalidArgument.class, invalidArgument -> {
+                assertThat(invalidArgument.e().error().parameters().field()).isEqualTo("field");


I can change this to .exception.

kunalrkak · 2025-10-28T16:47:50Z

...core/src/integrationInput/java/endpointerrors/com/palantir/product/ErrorServiceBlocking.java

+        record Unknown(RemoteException e) implements TestBasicErrorErrors {}
+    }
+
+    sealed interface TestImportedErrorErrors {


Singular definitely makes sense since the actual instance will be a single type. Also thoughts on ErrorType or something for the suffix? Was also thinking AssociatedError, but that feels like a mouthful

kunalrkak · 2025-10-28T16:52:16Z

...core/src/integrationInput/java/endpointerrors/com/palantir/product/ErrorServiceBlocking.java

+            if (e instanceof TestErrors.InvalidArgumentException ex) {
+                return new InvalidArgument(ex);
+            } else if (e instanceof ConjureErrors.ConflictingCauseSafeArgErrException ex) {
+                return new ConflictingCauseSafeArgErr(ex);


Aren't the series of instanceof checks unideal from a performance standpoint? If we were on source 21+ I would say a switch statement would be an easy replacement over a visitor

Do we have an option without instanceof in the absence of source 21+? Visitor patterns with overloads would have to rely on the RemoteException having an accept method to take a visitor, which simply won't work here 🤔

Yeah, I think we can actually match on the error name here. I can make that change.

kunalrkak · 2025-10-28T16:53:46Z

...core/src/integrationInput/java/endpointerrors/com/palantir/product/ErrorServiceBlocking.java

+            }
+        }
+
+        record InvalidArgument(TestErrors.InvalidArgumentException e) implements TestBasicErrorErrors {}


nit: we should name the parameter error or something so that the getter method is not .e()

kunalrkak · 2025-10-28T17:05:48Z

...re/src/main/java/com/palantir/conjure/java/services/dialogue/DialogueInterfaceGenerator.java

+        ClassName utiltyClassName = ClassName.get(
+                packageName,
+                className.simpleName(),
+                CaseFormat.LOWER_CAMEL.to(
+                                CaseFormat.UPPER_CAMEL,
+                                endpointDef.getEndpointName().get()) + "Errors");


I think you could use ClassName#nestedClass here

aldexis

Didn't look through the codegen pieces, but at the generated code. The new helpers seem like a good addition and shouldn't break anything.

The only ABI risks I could find are 1. having it adopted and reverting usage of the flag 2. Removing associations between errors and endpoints (even with the error still existing) I think this is probably fine.

I'm not entirely sure the abi checker would protect us from the removal in all cases however, specifically if the usage looks like

switch(TestImportedErrorErrors.from(exception)) {
  EndpointError -> log.error("Found an endpoint error");
  Unknown -> log.error("Found an unknown error");
}

where the class itself isn't used anywhere

Beyond this, I agree with Kunal's comments and will leave the deeper review to him

kunalrkak

Thanks!

Invalidated by push of 0e40e01

changelog-app · 2025-11-04T16:33:14Z

✅ Successfully generated changelog entry!

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.

📋Changelog Preview

💡 Improvements

Generate sealed interfaces to match against endpoint associated errors (#2696)

autorelease3 · 2025-11-05T20:21:11Z

Released 8.61.0

mpritham commented Oct 27, 2025

View reviewed changes

Pritham Marupaka added 4 commits October 27, 2025 18:26

wip: brainstorming target codegen

463df41

Add utility for endpoint associated errors

ab1eb98

Add unknown variant

007bb28

Add tests

65ee1b8

mpritham force-pushed the mpritham/endpoint-assoc-error-utility branch from 8c13809 to 65ee1b8 Compare October 27, 2025 22:27

mpritham commented Oct 27, 2025

View reviewed changes

formatting

182c899

mpritham requested review from aldexis, bjlaub and kunalrkak October 27, 2025 22:34

mpritham added the do not merge label Oct 27, 2025

mpritham commented Oct 28, 2025

View reviewed changes

kunalrkak reviewed Oct 28, 2025

View reviewed changes

aldexis reviewed Oct 30, 2025

View reviewed changes

Pritham Marupaka added 2 commits November 3, 2025 16:32

review changes

85a2c8b

make utility name singular

fea25dd

kunalrkak previously approved these changes Nov 3, 2025

View reviewed changes

fix test

0e40e01

nits

a99f7f8

mpritham added merge when ready autorelease and removed do not merge labels Nov 5, 2025

kunalrkak approved these changes Nov 5, 2025

View reviewed changes

bulldozer-bot bot merged commit f6afecb into develop Nov 5, 2025
5 checks passed

bulldozer-bot bot deleted the mpritham/endpoint-assoc-error-utility branch November 5, 2025 20:20


		record EndpointError(EndpointSpecificErrors.EndpointErrorException e) implements TestImportedErrorErrors {}

		record Unknown(RemoteException e) implements TestImportedErrorErrors {}

Conversation

mpritham commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before this PR

After this PR

Possible downsides?

Uh oh!

changelog-app bot commented Oct 27, 2025 • edited by mpritham Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generate changelog in changelog/@unreleased

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mpritham Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aldexis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kunalrkak left a comment

Choose a reason for hiding this comment

Uh oh!

changelog-app bot commented Nov 4, 2025

✅ Successfully generated changelog entry!

Need to regenerate?

📋Changelog Preview

💡 Improvements

Uh oh!

Uh oh!

autorelease3 bot commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mpritham commented Oct 27, 2025 •

edited

Loading

changelog-app bot commented Oct 27, 2025 •

edited by mpritham

Loading

Generate changelog in `changelog/@unreleased`

mpritham Oct 27, 2025 •

edited

Loading

aldexis left a comment •

edited

Loading