Skip to content

Conversation

@sttk
Copy link
Owner

@sttk sttk commented Apr 27, 2025

This PR adds an exception notification feature to this package.
When a Err object is instantiated, it notifies pre-registered exception handlers.
To enable this notification feature, the system property github.sttk.errs.notify=true must be specified at program startup.

@sttk sttk requested a review from Copilot April 27, 2025 09:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an exception notification feature for the package, enabling pre-registered exception handlers to be notified upon the creation of an exception object. Key changes include:

  • Updated test expectations for exception line numbers in ExcTest.java.
  • Added a new exception handler interface (ExcHandler.java) and its supporting functionality in Exc.java.
  • Introduced new tests (ExcHandlerTest.java) and updated related module and package configuration files.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/test/java/com/github/sttk/errs/ExcTest.java Updated expected line numbers in assertions to match new behavior.
src/test/java/com/github/sttk/errs/ExcHandlerTest.java Added tests to validate synchronous and asynchronous exception notification.
src/main/java/module-info.java Added a dependency on java.management to support notification features.
src/main/java/com/github/sttk/errs/package-info.java Adjusted header formatting in the file comments.
src/main/java/com/github/sttk/errs/ExcHandler.java Introduced a new functional interface for exception handling.
src/main/java/com/github/sttk/errs/Exc.java Implemented exception notification logic with handler registration and management.
Files not reviewed (1)
  • pom.xml: Language not supported
Comments suppressed due to low confidence (1)

src/main/java/com/github/sttk/errs/Exc.java:223

  • [nitpick] Consider renaming 'isFixed' to a more descriptive name such as 'handlersFixed' to better convey that it prevents further additions of handlers.
private static boolean isFixed = false;

@sttk sttk requested a review from Copilot April 27, 2025 09:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an exception notification feature that informs pre-registered exception handlers when an exception (Exc) is instantiated. Key changes include:

  • Updating test expectations for the exception’s file and line details.
  • Adding new APIs for registering synchronous and asynchronous exception handlers.
  • Integrating a notification mechanism into the Exc constructors and wiring it through a system property.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/test/java/com/github/sttk/errs/ExcTest.java Adjusted expected line numbers in tests to match updated stack trace details.
src/test/java/com/github/sttk/errs/ExcHandlerTest.java Added tests to verify addition, fixing, and notification of exception handlers.
src/main/java/module-info.java Added a requirement for java.management to support the new notification feature.
src/main/java/com/github/sttk/errs/package-info.java Reformatted comment lines.
src/main/java/com/github/sttk/errs/ExcHandler.java Introduces the functional interface for exception handling.
src/main/java/com/github/sttk/errs/Exc.java Enhanced to notify handlers on creation and added new methods for handler management.
Files not reviewed (1)
  • pom.xml: Language not supported
Comments suppressed due to low confidence (2)

src/test/java/com/github/sttk/errs/ExcTest.java:182

  • Verify that the exception message’s file and line details (now updated to line 180) correctly mirror the actual stack trace position after changes in the constructor.
assertThat(exc.toString()).isEqualTo("com.github.sttk.errs.Exc { reason = com.github.sttk.errs.ExcTest$IndexOutOfRange { name=data, index=4, min=0, max=3 }, file = ExcTest.java, line = 180 }");

src/test/java/com/github/sttk/errs/ExcTest.java:190

  • Ensure that the expected exception message now showing line 188 (for the cause case) correctly reflects the updated behavior.
assertThat(exc.toString()).isEqualTo("com.github.sttk.errs.Exc { reason = com.github.sttk.errs.ExcTest$IndexOutOfRange { name=data, index=4, min=0, max=3 }, file = ExcTest.java, line = 188, cause = java.lang.IndexOutOfBoundsException: Index out of range: 4 }");

@sttk sttk merged commit 4e7643b into main Apr 27, 2025
12 checks passed
@sttk sttk deleted the add_notification branch April 27, 2025 09:48
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.

2 participants