From c0585ac46859575c141431ad9d4125d78522b338 Mon Sep 17 00:00:00 2001 From: Adrien S Date: Fri, 19 Apr 2024 02:59:49 +0200 Subject: [PATCH] Store error backtraces on discrete executions (#1325) * Store error backtraces on discrete executions * Update app/models/good_job/execution.rb Co-authored-by: Ben Sheldon [he/him] * Remove duplicated backtrace formatting and add to job#show page * Make error color red too in jobs table --------- Co-authored-by: Ben Sheldon [he/him] Co-authored-by: Ben Sheldon [he/him] --- Gemfile.lock | 3 +++ app/models/good_job/discrete_execution.rb | 7 +++++++ app/models/good_job/execution.rb | 8 ++++++++ app/views/good_job/jobs/_executions.erb | 7 ++++++- app/views/good_job/jobs/_table.erb | 2 +- ...4_create_good_job_execution_error_backtrace.rb | 15 +++++++++++++++ demo/db/schema.rb | 3 ++- .../install/migrations/create_good_jobs.rb.erb | 1 + ...eate_good_job_execution_error_backtrace.rb.erb | 15 +++++++++++++++ spec/app/jobs/example_job_spec.rb | 1 + 10 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 demo/db/migrate/20240417093204_create_good_job_execution_error_backtrace.rb create mode 100644 lib/generators/good_job/templates/update/migrations/10_create_good_job_execution_error_backtrace.rb.erb diff --git a/Gemfile.lock b/Gemfile.lock index eb0c54116..cfab89e1d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -277,6 +277,8 @@ GEM racc (~> 1.4) nokogiri (1.16.2-java) racc (~> 1.4) + nokogiri (1.16.2-x86_64-darwin) + racc (~> 1.4) nokogiri (1.16.2-x86_64-linux) racc (~> 1.4) octokit (4.25.1) @@ -487,6 +489,7 @@ PLATFORMS arm64-darwin-23 universal-java-11 universal-java-20 + x86_64-darwin-23 x86_64-linux DEPENDENCIES diff --git a/app/models/good_job/discrete_execution.rb b/app/models/good_job/discrete_execution.rb index 5ad3e3c74..ed0ff77bd 100644 --- a/app/models/good_job/discrete_execution.rb +++ b/app/models/good_job/discrete_execution.rb @@ -21,6 +21,13 @@ def self.error_event_migrated? false end + def self.backtrace_migrated? + return true if columns_hash["error_backtrace"].present? + + migration_pending_warning! + false + end + def number serialized_params.fetch('executions', 0) + 1 end diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index c599e3694..d6d83d7bc 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -362,6 +362,10 @@ def self.format_error(error) [error.class.to_s, ERROR_MESSAGE_SEPARATOR, error.message].join end + def self.format_backtrace(backtrace) + Rails.backtrace_cleaner.clean(backtrace || []) + end + # Execute the ActiveJob job this {Execution} represents. # @return [ExecutionResult] # An array of the return value of the job's +#perform+ method and the @@ -461,6 +465,10 @@ def perform if discrete_execution discrete_execution.error = error_string discrete_execution.error_event = result.error_event + if discrete_execution.class.backtrace_migrated? + error_backtrace = self.class.format_backtrace(job_error.backtrace) + discrete_execution.error_backtrace = error_backtrace + end end else job_attributes[:error] = nil diff --git a/app/views/good_job/jobs/_executions.erb b/app/views/good_job/jobs/_executions.erb index 04792b041..ecdec0309 100644 --- a/app/views/good_job/jobs/_executions.erb +++ b/app/views/good_job/jobs/_executions.erb @@ -35,9 +35,14 @@ <% if execution.error %>
- <%=t "good_job.shared.error" %>: + <%=t "good_job.shared.error" %>: <%= execution.error %>
+ <% if GoodJob::DiscreteExecution.backtrace_migrated? && execution.error_backtrace&.any? %> +
+ <%= execution.error_backtrace[0] %> +
+ <% end %> <% end %> <% end %> <%= render 'good_job/custom_execution_details', execution: execution, job: @job %> diff --git a/app/views/good_job/jobs/_table.erb b/app/views/good_job/jobs/_table.erb index e7cb84a10..2ff3c4072 100644 --- a/app/views/good_job/jobs/_table.erb +++ b/app/views/good_job/jobs/_table.erb @@ -68,7 +68,7 @@ <%= tag.h5 tag.code(link_to(job.display_name, job_path(job), class: "text-reset text-decoration-none")), class: "text-reset mb-0" %> <% if job.error %>
- <%=t "good_job.shared.error" %>: + <%=t "good_job.shared.error" %>: <%= job.error %>
<% end %> diff --git a/demo/db/migrate/20240417093204_create_good_job_execution_error_backtrace.rb b/demo/db/migrate/20240417093204_create_good_job_execution_error_backtrace.rb new file mode 100644 index 000000000..6b054b640 --- /dev/null +++ b/demo/db/migrate/20240417093204_create_good_job_execution_error_backtrace.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateGoodJobExecutionErrorBacktrace < ActiveRecord::Migration[7.1] + def change + reversible do |dir| + dir.up do + # Ensure this incremental update migration is idempotent + # with monolithic install migration. + return if connection.column_exists?(:good_job_executions, :error_backtrace) + end + end + + add_column :good_job_executions, :error_backtrace, :text, array: true + end +end diff --git a/demo/db/schema.rb b/demo/db/schema.rb index 2100422ee..387a6e030 100644 --- a/demo/db/schema.rb +++ b/demo/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2024_01_14_221613) do +ActiveRecord::Schema.define(version: 2024_04_17_093204) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -41,6 +41,7 @@ t.datetime "finished_at" t.text "error" t.integer "error_event", limit: 2 + t.text "error_backtrace", array: true t.index ["active_job_id", "created_at"], name: "index_good_job_executions_on_active_job_id_and_created_at" end diff --git a/lib/generators/good_job/templates/install/migrations/create_good_jobs.rb.erb b/lib/generators/good_job/templates/install/migrations/create_good_jobs.rb.erb index 0d3bd40f0..dce96234f 100644 --- a/lib/generators/good_job/templates/install/migrations/create_good_jobs.rb.erb +++ b/lib/generators/good_job/templates/install/migrations/create_good_jobs.rb.erb @@ -57,6 +57,7 @@ class CreateGoodJobs < ActiveRecord::Migration<%= migration_version %> t.datetime :finished_at t.text :error t.integer :error_event, limit: 2 + t.text :error_backtrace, array: true end create_table :good_job_processes, id: :uuid do |t| diff --git a/lib/generators/good_job/templates/update/migrations/10_create_good_job_execution_error_backtrace.rb.erb b/lib/generators/good_job/templates/update/migrations/10_create_good_job_execution_error_backtrace.rb.erb new file mode 100644 index 000000000..ffcde1558 --- /dev/null +++ b/lib/generators/good_job/templates/update/migrations/10_create_good_job_execution_error_backtrace.rb.erb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateGoodJobExecutionErrorBacktrace < ActiveRecord::Migration<%= migration_version %> + def change + reversible do |dir| + dir.up do + # Ensure this incremental update migration is idempotent + # with monolithic install migration. + return if connection.column_exists?(:good_job_executions, :error_backtrace) + end + end + + add_column :good_job_executions, :error_backtrace, :text, array: true + end +end diff --git a/spec/app/jobs/example_job_spec.rb b/spec/app/jobs/example_job_spec.rb index 8706c76d2..e3cbb93a1 100644 --- a/spec/app/jobs/example_job_spec.rb +++ b/spec/app/jobs/example_job_spec.rb @@ -60,6 +60,7 @@ good_job = GoodJob::Job.find_by(active_job_id: active_job.job_id) expect(good_job.discrete_executions.count).to eq 3 expect(good_job.discrete_executions.last.error).to be_present + expect(good_job.discrete_executions.last.error_backtrace).to eq(["app/jobs/example_job.rb:41:in `perform'"]) end end