[do-not-merge] Deserialize endpoint errors #2443
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generate changelog in
|
mpritham
commented
Dec 5, 2024
| @Safe String errorCode, | ||
| @Safe String errorName, | ||
| @Safe String errorInstanceId, | ||
| @Safe String arg, |
There was a problem hiding this comment.
- can put the args into a separate record which includes matches the
parametersmap for deserialization so we don't need a custom deserializer. - let's annotate each of the parameter fields with a
@JsonProperty
mpritham
commented
Dec 5, 2024
...c/test/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorsConjureBodySerDeTest.java
Outdated
Show resolved
Hide resolved
mpritham
commented
Dec 5, 2024
dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java
Outdated
Show resolved
Hide resolved
mpritham
force-pushed
the
pm/endpoint-error-serde
branch
5 times, most recently
from
6081541 to
74008d5
Compare
added 2 commits
December 12, 2024 13:13
mpritham
force-pushed
the
pm/endpoint-error-serde
branch
from
74008d5 to
a46dd8b
Compare
mpritham
commented
Jan 7, 2025
| @@ -67,31 +75,49 @@ final class ConjureBodySerDe implements BodySerDe { | |||
| */ | |||
| ConjureBodySerDe( | |||
| List<WeightedEncoding> rawEncodings, | |||
| ErrorDecoder errorDecoder, | |||
| ErrorDecoder _errorDecoder, | |||
There was a problem hiding this comment.
We don't need to pass this in. DefaultConjureRuntime is the public API where this ctor is created, and it passes in a hardcoded ErrorDecoder.INSTANCE.
mpritham
force-pushed
the
pm/endpoint-error-serde
branch
from
a46dd8b to
c6dfe0c
Compare
mpritham
commented
Jan 7, 2025
| errorDecoder = mock(ErrorDecoder.class); | ||
| when(errorDecoder.isError(response)).thenReturn(true); | ||
| when(errorDecoder.decode(response)).thenReturn(new RemoteException(serialized, 400)); | ||
| TestResponse response = TestResponse.withBody( |
There was a problem hiding this comment.
This is to have an "e2e" test where the error decoder is actually called (instead of mocked) - to validate that replacing the EncodingDeserializerRegistry with EncodingDeserializerForEndpointRegistry doesn't introduce a regression.
I will also port over the tests from ErrorDecoderTest to test the new EndpointErrorDecoder
mpritham
commented
Jan 7, 2025
| @@ -18,6 +18,7 @@ | |||
|
|
|||
| import com.palantir.dialogue.Response; | |||
|
|
|||
| // TODO(pm): use the new EndpointErrorDecoder | |||
There was a problem hiding this comment.
Replace impl in ErrorDecoder to delegate to the empty-map initialized EndpointErrorDecoder
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this PR
This is a prototype PR and should not be merged.
We'd like to support the a new method in
BodySerDethat allows the creation of deserializers given an expected result type, and a set of expected error types.A ConjureError is serialized and sent by servers. Dialogue deserializes the errors to a
SerializableErrorwhich loses the type information of the parameters. By providing the type information of the parameters, Dialogue can deserialize the receivedConjureErrorinto the well-typed objects (instead of Strings).After this PR
==COMMIT_MSG==
==COMMIT_MSG==
Possible downsides?