Untracked: Rethrow UserErrors in exceptions #90
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates exception handling in data merging/stacking operations to preserve UserError classes when re-throwing exceptions. The changes ensure that user-facing errors maintain their proper error type while preventing false positives in error monitoring systems.
- Refactored the
createExceptionHandlerfunction to detect and preserveUserErrortypes - Added import for
StopForUserErrorfrom the flipU package - Updated version number to reflect these changes
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| R/mergingandstackingutilities.R | Refactored exception handler to preserve UserError types and use appropriate error functions |
| DESCRIPTION | Incremented version number from 1.8.9 to 1.8.10 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| condition.fun <- function(...) { | ||
| if (warn) { | ||
| warning(..., call. = FALSE) | ||
| } else if (inherits(e, "UserError")) { | ||
| StopForUserError(...) | ||
| } else { | ||
| stop(..., call. = FALSE) |
There was a problem hiding this comment.
The condition.fun closure captures the exception e from the outer scope, but this creates a dependency on the outer variable that may not be the intended behavior. When condition.fun is called with different messages, it still checks inherits(e, "UserError") based on the original exception, which may not match the current context where condition.fun is being invoked.
| condition.fun <- function(...) { | |
| if (warn) { | |
| warning(..., call. = FALSE) | |
| } else if (inherits(e, "UserError")) { | |
| StopForUserError(...) | |
| } else { | |
| stop(..., call. = FALSE) | |
| condition.fun <- function(msg) { | |
| if (warn) { | |
| warning(msg, call. = FALSE) | |
| } else { | |
| if (inherits(e, "UserError")) { | |
| StopForUserError(msg) | |
| } else { | |
| stop(msg, call. = FALSE) | |
| } |
| condition.fun <- function(...) { | ||
| if (warn) { | ||
| warning(..., call. = FALSE) | ||
| } else if (inherits(e, "UserError")) { | ||
| StopForUserError(...) | ||
| } else { | ||
| stop(..., call. = FALSE) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is the actual change. Some minor differences below since the call. argument is already specified here.
Preserve UserError class when re-throwing exceptions in data merging/stacking operations, ensuring proper error handling and user-facing error messages and preventing stop conditions and generating false positives for unexpected errors in bugdup.