Skip to content

Conversation

@dummycode
Copy link

@dummycode dummycode commented Jul 10, 2025

Before

Struggled to write an assert in #1366 without duplicating a lot of code. Also, we had to create a ServiceOrEndpointServiceException in our repo so that we could handle both ServiceExceptions and EndpointServiceExceptions as the same thing, because they pretty much are.

After

  • Both ServiceException and EndpointServiceException extend BaseServiceException

Questions

I decided to make an abstract class vs. having EndpointServiceException extend ServiceException since ServiceException was final. Thoughts on making it non-final?

@changelog-app
Copy link

changelog-app bot commented Jul 10, 2025

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

Add abstract BaseServiceException as ancestor of ServiceException and EndpointServiceException

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@mpritham mpritham left a comment

Choose a reason for hiding this comment

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

After making EndpointServiceException abstract, I'd like to test out generating errors in conjure-java with this change to validate we haven't broken anything.

* from a service endpoint.
*/
public abstract class EndpointServiceException extends RuntimeException implements SafeLoggable {
public final class EndpointServiceException extends BaseServiceException implements SafeLoggable {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be abstract. EndpointServiceExceptions should not be initialized. They are extended by a specific endpoint associated error (e.g. here)

import java.util.List;
import javax.annotation.Nullable;

public abstract class BaseServiceException extends RuntimeException {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should be public. We don't expect any exceptions outside of ServiceException and EndpointServiceException to use this.

@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