Skip to content

Conversation

@dummycode
Copy link

@dummycode dummycode commented Jun 30, 2025

We began tagging errors as endpoint errors and thus wish to cleanly swap out assertServiceExceptionThrownBy with assertEndpointServiceExceptionThrownBy. I have duplicated this utility temporarily within OMS, but want to push this down into the library.

I did not see any easy way to combine EndpointServiceExceptionAssert and ServiceExceptionAssert since there is no relation between ServiceException and EndpointServiceException. Thoughts?

@changelog-app

This comment was marked as resolved.

}

private static void assertPut(Map<String, Object> map, String key, Object value, String name) {
Object previous = map.put(key, value);
Copy link
Member

Choose a reason for hiding this comment

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

Mutable records is sort of an anti-pattern. I would just make this a normal class.

IMO this class just feels like unnecessary indirection. We have a class that contains two fields, only to ever consume each field independently.

Copy link
Author

Choose a reason for hiding this comment

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

Ack, would revert back to normal class. I just extracted this from the existing class, but happy to tweak things.

@mpritham is going to take this over since they implemented the new error type. It was suggested the two exceptions should probably share an interface or ancestor so we can de-duplicate a lot of this logic.

@stale
Copy link

stale bot commented Oct 18, 2025

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

@stale stale bot added the stale label Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants