-
-
Notifications
You must be signed in to change notification settings - Fork 76
Add handle_exception method to log errors during GenerationJob execution #310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,18 @@ module Rescue | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| include ActiveSupport::Rescuable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class_methods do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Handles exceptions raised during GenerationJob execution. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Called by GenerationJob#handle_exception_with_agent_class as a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # class-level fallback when no instance is available to handle the error. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # @param exception [Exception] the exception to handle | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # @return [void] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def handle_exception(exception) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Rails.logger.error "[#{name}] #{exception.class}: #{exception.message}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Rails.logger.error exception.backtrace&.first(10)&.join("\n") if exception.backtrace | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+26
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+27
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Rails.logger.error "[#{name}] #{exception.class}: #{exception.message}" | |
| Rails.logger.error exception.backtrace&.first(10)&.join("\n") if exception.backtrace | |
| end | |
| agent_logger.error "[#{name}] #{exception.class}: #{exception.message}" | |
| agent_logger.error exception.backtrace&.first(10)&.join("\n") if exception.backtrace | |
| end | |
| private | |
| # Returns the logger to use for class-level exception handling. | |
| # | |
| # Prefers the including class's logger, then ActiveAgent::Base.logger, | |
| # then Rails.logger if available, and finally falls back to a standard | |
| # Ruby Logger to $stderr. | |
| # | |
| # @return [#error] a logger-like object responding to `#error` | |
| def agent_logger | |
| if respond_to?(:logger) && (current_logger = logger) | |
| current_logger | |
| elsif defined?(ActiveAgent::Base) && | |
| ActiveAgent::Base.respond_to?(:logger) && | |
| (base_logger = ActiveAgent::Base.logger) | |
| base_logger | |
| elsif defined?(Rails) && Rails.respond_to?(:logger) && Rails.logger | |
| Rails.logger | |
| else | |
| require "logger" | |
| @__active_agent_rescue_logger ||= Logger.new($stderr) | |
| end | |
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As written, this default
handle_exceptionimplementation will swallow the original exception (ActiveJobrescue_fromconsiders it handled), which can cause GenerationJob failures to be silently marked successful and prevent job-level retries/discard logic from running. Consider re-raising after logging (or making the default behavior opt-in) so failures still surface to the job framework.