Skip to content

Ensure endpoint associated error serialization follows the Conjure spec#2402

Merged
mpritham merged 16 commits intopm/endpoint-errorsfrom
pm/endpoint-error-serialization
Nov 18, 2024
Merged

Ensure endpoint associated error serialization follows the Conjure spec#2402
mpritham merged 16 commits intopm/endpoint-errorsfrom
pm/endpoint-error-serialization

Conversation

@mpritham
Copy link
Contributor

@mpritham mpritham commented Nov 8, 2024

Before this PR

Builds on #2401. Serialize the arguments in CheckedServiceExceptions as ConjureErrors

After this PR

==COMMIT_MSG==
==COMMIT_MSG==

Possible downsides?

@mpritham mpritham force-pushed the pm/endpoint-error-serialization branch from aa2b4b2 to 768274e Compare November 8, 2024 18:28
@mpritham mpritham force-pushed the pm/endpoint-errors branch 3 times, most recently from 02535ab to a573b92 Compare November 13, 2024 17:01
@mpritham mpritham force-pushed the pm/endpoint-error-serialization branch 2 times, most recently from 073ab94 to 37c970f Compare November 13, 2024 18:43
@mpritham mpritham force-pushed the pm/endpoint-error-serialization branch from 37c970f to 70cc1c1 Compare November 13, 2024 18:44
@mpritham mpritham force-pushed the pm/endpoint-error-serialization branch from e1d15a6 to 9125ce1 Compare November 13, 2024 18:52
@mpritham mpritham force-pushed the pm/endpoint-error-serialization branch from 9125ce1 to 8ab8190 Compare November 13, 2024 18:53
@mpritham mpritham force-pushed the pm/endpoint-error-serialization branch 6 times, most recently from e63f11a to 1ba0c36 Compare November 14, 2024 17:08
@mpritham mpritham force-pushed the pm/endpoint-error-serialization branch from 1ba0c36 to ab0b351 Compare November 14, 2024 18:15
carterkozak
carterkozak previously approved these changes Nov 15, 2024
Copy link
Contributor

@carterkozak carterkozak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@mpritham mpritham marked this pull request as ready for review November 18, 2024 16:21
@mpritham mpritham changed the title wip: Endpoint error serialization Endpoint associated error serialization follows the Conjure spec Nov 18, 2024
@mpritham mpritham changed the title Endpoint associated error serialization follows the Conjure spec Ensure endpoint associated error serialization follows the Conjure spec Nov 18, 2024
ConjureError error = CLIENT_JSON_MAPPER.readValue(getErrorBody(connection), ConjureError.class);
assertThat(error.errorName()).isEqualTo("Default:Conflict");
assertThat(error.errorCode()).isEqualTo("CONFLICT");
assertThat(error.parameters()).containsOnlyKeys("optionalInt", "arg", "optionalDouble", "optional");
Copy link
Contributor Author

@mpritham mpritham Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notably the keys nullValue, emptyOptionalInt, emptyOptionalDouble, and emptyOptional are not in the map as expected.

@policy-bot policy-bot bot dismissed carterkozak’s stale review November 18, 2024 17:23

Invalidated by push of 667d310

@mpritham mpritham force-pushed the pm/endpoint-error-serialization branch from 1490b3f to 40fbfe3 Compare November 18, 2024 18:47
@mpritham mpritham merged commit d02a8b9 into pm/endpoint-errors Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants