-
Notifications
You must be signed in to change notification settings - Fork 43
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
Support endpoint errors #2401
base: develop
Are you sure you want to change the base?
Support endpoint errors #2401
Conversation
Generate changelog in
|
ac2bcd2
to
e148180
Compare
conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowErrorService.java
Outdated
Show resolved
Hide resolved
...ava-core/src/integrationInput/java/com/palantir/product/ServerEndpointSpecificTwoErrors.java
Outdated
Show resolved
Hide resolved
bf0d2aa
to
86cf0f3
Compare
86cf0f3
to
4db6495
Compare
1b468d2
to
83c7ea0
Compare
This is good for a re-review. Shouldn't be merged: it has the CheckedServiceException class which we'd like to be in the conjure-java-runtime-api repo, it uses an RC version of conjure. Going to branch off of this to work on the error serialization piece. |
conjure-java-core/src/integrationInput/java/com/palantir/another/ServerConjureErrors.java
Outdated
Show resolved
Hide resolved
...a-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceHandlerGenerator.java
Outdated
Show resolved
Hide resolved
conjure-java-core/src/main/java/com/palantir/conjure/java/util/ErrorGenerationUtils.java
Outdated
Show resolved
Hide resolved
f49fa60
to
673f89d
Compare
conjure-java-core/src/main/java/com/palantir/conjure/java/util/ErrorGenerationUtils.java
Outdated
Show resolved
Hide resolved
673f89d
to
811e1ef
Compare
...core/src/main/java/com/palantir/conjure/java/services/UndertowServiceInterfaceGenerator.java
Outdated
Show resolved
Hide resolved
...core/src/main/java/com/palantir/conjure/java/services/UndertowServiceInterfaceGenerator.java
Outdated
Show resolved
Hide resolved
conjure-java-core/src/main/java/com/palantir/conjure/java/util/Javadoc.java
Outdated
Show resolved
Hide resolved
...core/src/main/java/com/palantir/conjure/java/services/UndertowServiceInterfaceGenerator.java
Outdated
Show resolved
Hide resolved
a59df32
to
d003482
Compare
...e-java-core/src/integrationInput/java/com/palantir/another/ServerEndpointSpecificErrors.java
Outdated
Show resolved
Hide resolved
|
||
public static Optional<String> getJavaDoc(EndpointDefinition endpointDef) { | ||
return getJavaDocInternal(endpointDef, false); | ||
public static void addJavaDocForEndpointDefinition( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introduced this method which adds the javadoc to the provided methodBuilder
. I opted to add an "options" record because there are two dimensions of generation options we need to support: 1) if we should include the request line (e.g. @apiNote {@code GET /foo/bar}
), 2) if we should include endpoint errors.
Usage | Request Line? | Endpoint Errors? |
---|---|---|
Jersey service generation | No | No |
Undertow service generation | Yes | Yes |
Dialogue interface generation | Yes | No* |
*Not yet at least. We can either decide to return a result type in Dialogue, or throw a checked exception. If we do the latter, we need to add @throws
pieces to the JavaDoc similar to what we do when generating Undertow service interfaces. If we go with the former, we'll still need to document the result union type but the JavaDoc would be added where we define the results type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Jersey service generation, we shouldn't even get to the logic that adds the JavaDoc. Endpoint errors are not supported for Jersey, and we will throw an IllegalArgumentException
if a user attempts to code-gen Jersey services with endpoint errors defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I think we always want to add the error javadoc if errors are present -- we validate that they aren't when the legacy Jersey generator is used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, sorry, missed the piece about dialogue, definitely don't want to leak the server-side piece there!
d003482
to
32306c8
Compare
conjure-java-core/src/main/java/com/palantir/conjure/java/services/ServiceGenerators.java
Outdated
Show resolved
Hide resolved
32306c8
to
02535ab
Compare
02535ab
to
a573b92
Compare
Before this PR
Endpoint errors were added to the Conjure specification in this PR. conjure-java does not use user-defined endpoint errors.
After this PR
Generated
ServerErrors
classesDevelopers will be able to create checked exceptions for each error that is defined as an error an endpoint can throw. Specifically, for each endpoint-defined error,
conjure-java
will generate a class for the error's namespace with name{namespace}ServerErrors
. This class will hold the definition of an exception, which is a sub-class ofCheckedServiceException
(introduced here), as well as methods to construct the exception.Changes to
Errors
classesWhen an error is used as an endpoint error, factory methods to create
ServiceException
s are no longer provided in theErrors
utility classes. 🌶️ This means that once an error becomes associated with an endpoint, it can no longer be used to throw un-checked runtime exceptions. This is intentional and acts as a forcing function for adoption: associating errors with one endpoint will require users to add endpoint errors for all of the endpoints throwing the error.Updates to Undertow Service Interfaces and Handlers
For each endpoint that defines endpoint-errors, the corresponding endpoint in the Undertow service interface, and the endpoint handler method, will include a
throws
clause including the list of checked exceptions corresponding to the endpoint errors, that were generated in theServerErrors
class (see above).Jersey
Endpoint errors will not be supported on Jersey servers. An exception will be thrown if a user attempts to generate code for Jersey services with endpoint errors defined.
==COMMIT_MSG==
Support endpoint errors
==COMMIT_MSG==
Possible downsides?