From 78432125a5747d9967b9ad6752dbee889bd369e8 Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Tue, 16 Nov 2021 09:23:46 -0500
Subject: [PATCH 1/9] Initial implementation

---
 lib/delayed/job_groups/job_group.rb       | 27 ++++++++-
 spec/delayed/job_groups/job_group_spec.rb | 74 ++++++++++++++++++++---
 2 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/lib/delayed/job_groups/job_group.rb b/lib/delayed/job_groups/job_group.rb
index d1bdb1c..1846d2b 100644
--- a/lib/delayed/job_groups/job_group.rb
+++ b/lib/delayed/job_groups/job_group.rb
@@ -48,7 +48,19 @@ def unblock
       end
 
       def cancel
-        Delayed::Job.enqueue(on_cancellation_job, on_cancellation_job_options || {}) if on_cancellation_job
+        job = nil
+        job_options = nil
+
+        # Deserialization of the job or its options can fail
+        begin
+          job = on_cancellation_job
+          job_options = on_cancellation_job_options
+        rescue StandardError
+          Delayed::Worker.logger.info("Failed to deserialize the on_cancellation_job or on_cancellation_job_options for job_group_id=#{id}. Skipping on_cancellation_job to clean up job group.")
+        end
+
+        Delayed::Job.enqueue(job, job_options || {}) if job
+
         destroy
       end
 
@@ -80,7 +92,18 @@ def ready_for_completion?
       end
 
       def complete
-        Delayed::Job.enqueue(on_completion_job, on_completion_job_options || {}) if on_completion_job
+        job = nil
+        job_options = nil
+
+        # Deserialization of the job or its options can fail
+        begin
+          job = on_completion_job
+          job_options = on_completion_job_options
+        rescue StandardError
+          Delayed::Worker.logger.info("Failed to deserialize the on_completion_job or on_completion_job_options for job_group_id=#{id}. Skipping on_completion_job to clean up job group.")
+        end
+
+        Delayed::Job.enqueue(job, job_options || {}) if job
         destroy
       end
     end
diff --git a/spec/delayed/job_groups/job_group_spec.rb b/spec/delayed/job_groups/job_group_spec.rb
index e142954..a06d518 100644
--- a/spec/delayed/job_groups/job_group_spec.rb
+++ b/spec/delayed/job_groups/job_group_spec.rb
@@ -140,6 +140,31 @@
         expect(job_group).to have_been_destroyed
       end
     end
+
+    context "on_completion_job refers to missing class" do
+      module Delayed::JobGroups::JobGroupTestHelper
+        class OnCompletionJob
+
+        end
+      end
+
+      it "handles missing on_completion_job" do
+        job_group = Delayed::JobGroups::JobGroup.create!(on_completion_job: Delayed::JobGroups::JobGroupTestHelper::OnCompletionJob.new,
+                                                         on_completion_job_options: {})
+        job = Delayed::Job.create!(job_group_id: job_group.id)
+        job_group.mark_queueing_complete
+        job.destroy
+
+        # Remove the class for on_completion_job
+        Delayed::JobGroups::JobGroupTestHelper.module_eval do
+          remove_const 'OnCompletionJob'
+        end
+
+        # Deserialization fails
+        expect { Delayed::JobGroups::JobGroup.check_for_completion(job_group.id) }.not_to raise_error
+        expect(job_group).to have_been_destroyed
+      end
+    end
   end
 
   describe "#enqueue" do
@@ -212,20 +237,49 @@
     let!(:queued_job) { Delayed::Job.create!(job_group_id: job_group.id) }
     let!(:running_job)  { Delayed::Job.create!(job_group_id: job_group.id, locked_at: Time.now, locked_by: 'test') }
 
-    before do
-      job_group.cancel
-    end
+    context "with no on_cancellation_job" do
+      before do
+        job_group.cancel
+      end
 
-    it "destroys the job group" do
-      expect(job_group).to have_been_destroyed
-    end
+      it "destroys the job group" do
+        expect(job_group).to have_been_destroyed
+      end
 
-    it "destroys queued jobs" do
-      expect(queued_job).to have_been_destroyed
+      it "destroys queued jobs" do
+        expect(queued_job).to have_been_destroyed
+      end
+
+      it "does not destroy running jobs" do
+        expect(running_job).not_to have_been_destroyed
+      end
     end
 
-    it "does not destroy running jobs" do
-      expect(running_job).not_to have_been_destroyed
+    context "on_cancellation_job refers to missing class" do
+      module Delayed::JobGroups::JobGroupTestHelper
+        class OnCancellationJob
+
+        end
+      end
+
+      subject(:job_group) do
+        Delayed::JobGroups::JobGroup.create!(on_cancellation_job: Delayed::JobGroups::JobGroupTestHelper::OnCancellationJob.new,
+                                             on_cancellation_job_options: {})
+      end
+
+      it "handles missing on_cancellation_job" do
+        job_group = Delayed::JobGroups::JobGroup.create!(on_cancellation_job: Delayed::JobGroups::JobGroupTestHelper::OnCancellationJob.new,
+                                                         on_cancellation_job_options: {})
+
+        # Remove the class for on_cancellation_job
+        Delayed::JobGroups::JobGroupTestHelper.module_eval do
+          remove_const 'OnCancellationJob'
+        end
+
+        # Deserialization fails
+        expect { job_group.cancel }.not_to raise_error
+        expect(job_group).to have_been_destroyed
+      end
     end
   end
 

From 844e50688d13bb96ab3f98b9642034fa0cdb0295 Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Tue, 16 Nov 2021 09:58:56 -0500
Subject: [PATCH 2/9] Add configurable error_reporter

---
 README.md                                 | 16 ++++++++++++++-
 lib/delayed/job_groups/configuration.rb   | 13 ++++++++++++
 lib/delayed/job_groups/job_group.rb       | 10 ++++++++--
 lib/delayed_job_groups_plugin.rb          | 17 ++++++++++++++++
 spec/delayed/job_groups/job_group_spec.rb | 24 +++++++++++++++++++++++
 5 files changed, 77 insertions(+), 3 deletions(-)
 create mode 100644 lib/delayed/job_groups/configuration.rb

diff --git a/README.md b/README.md
index 7fdd6f7..180e68d 100644
--- a/README.md
+++ b/README.md
@@ -101,10 +101,24 @@ job_group.cancel
 Configuration to allow failed jobs not to cancel the group
 ```ruby
 # We can optionally pass options that will allow jobs to fail without cancelling the group.
-# This also allows the on_completion job to fire once all jobs have either succeeded or failed. 
+# This also allows the on_completion job to fire once all jobs have either succeeded or failed.
 job_group = Delayed::JobGroups::JobGroup.create!(failure_cancels_group: false)
 ```
 
+### Job Group Plugin Options
+
+The job group plugin can be configured in an initializer (e.g. `config/initializers/delayed_job_groups_plugin.rb`) as follows:
+
+```ruby
+Delayed::JobGroups.configure do |configuration|
+  configuration.error_reporter = Proc.new { |error| Bugsnag.notify(error) }
+end
+```
+
+The plugin supports the following options (all of which are optional):
+
+* `error_reporter` - a callback proc that accepts an `Exception` if the plugin encounters an unexpected error. This can be useful for reporting to an error monitoring system.
+
 ## Supported Platforms
 
 * Only the Delayed Job Active Record backend is supported.
diff --git a/lib/delayed/job_groups/configuration.rb b/lib/delayed/job_groups/configuration.rb
new file mode 100644
index 0000000..af38ebb
--- /dev/null
+++ b/lib/delayed/job_groups/configuration.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+require 'delayed_job'
+require 'set'
+
+module Delayed
+  module JobGroups
+    class Configuration
+      attr_accessor :error_reporter
+    end
+  end
+end
+
diff --git a/lib/delayed/job_groups/job_group.rb b/lib/delayed/job_groups/job_group.rb
index 1846d2b..6171789 100644
--- a/lib/delayed/job_groups/job_group.rb
+++ b/lib/delayed/job_groups/job_group.rb
@@ -55,8 +55,9 @@ def cancel
         begin
           job = on_cancellation_job
           job_options = on_cancellation_job_options
-        rescue StandardError
+        rescue StandardError => e
           Delayed::Worker.logger.info("Failed to deserialize the on_cancellation_job or on_cancellation_job_options for job_group_id=#{id}. Skipping on_cancellation_job to clean up job group.")
+          error_reporter.call(e) if error_reporter
         end
 
         Delayed::Job.enqueue(job, job_options || {}) if job
@@ -99,13 +100,18 @@ def complete
         begin
           job = on_completion_job
           job_options = on_completion_job_options
-        rescue StandardError
+        rescue StandardError => e
           Delayed::Worker.logger.info("Failed to deserialize the on_completion_job or on_completion_job_options for job_group_id=#{id}. Skipping on_completion_job to clean up job group.")
+          error_reporter.call(e) if error_reporter
         end
 
         Delayed::Job.enqueue(job, job_options || {}) if job
         destroy
       end
+
+      def error_reporter
+        Delayed::JobGroups.configuration.error_reporter
+      end
     end
   end
 end
diff --git a/lib/delayed_job_groups_plugin.rb b/lib/delayed_job_groups_plugin.rb
index acbaa26..4077d01 100644
--- a/lib/delayed_job_groups_plugin.rb
+++ b/lib/delayed_job_groups_plugin.rb
@@ -4,6 +4,7 @@
 require 'active_record'
 require 'delayed_job'
 require 'delayed_job_active_record'
+require 'delayed/job_groups/configuration'
 require 'delayed/job_groups/compatibility'
 require 'delayed/job_groups/job_extensions'
 require 'delayed/job_groups/job_group'
@@ -20,3 +21,19 @@
 end
 
 Delayed::Worker.plugins << Delayed::JobGroups::Plugin
+
+module Delayed
+  module JobGroups
+    @configuration = Delayed::JobGroups::Configuration.new
+
+    class << self
+      def configure
+        yield(configuration) if block_given?
+      end
+
+      def configuration
+        @configuration
+      end
+    end
+  end
+end
diff --git a/spec/delayed/job_groups/job_group_spec.rb b/spec/delayed/job_groups/job_group_spec.rb
index a06d518..741acbe 100644
--- a/spec/delayed/job_groups/job_group_spec.rb
+++ b/spec/delayed/job_groups/job_group_spec.rb
@@ -148,6 +148,17 @@ class OnCompletionJob
         end
       end
 
+      let(:error_reporter) { Proc.new { |_error| } }
+
+      around do |example|
+        original_error_reporter = Delayed::JobGroups.configuration.error_reporter
+        Delayed::JobGroups.configuration.error_reporter = error_reporter
+        example.run
+        Delayed::JobGroups.configuration.error_reporter = original_error_reporter
+      end
+
+      before { allow(error_reporter).to receive(:call) }
+
       it "handles missing on_completion_job" do
         job_group = Delayed::JobGroups::JobGroup.create!(on_completion_job: Delayed::JobGroups::JobGroupTestHelper::OnCompletionJob.new,
                                                          on_completion_job_options: {})
@@ -162,6 +173,7 @@ class OnCompletionJob
 
         # Deserialization fails
         expect { Delayed::JobGroups::JobGroup.check_for_completion(job_group.id) }.not_to raise_error
+        expect(error_reporter).to have_received(:call)
         expect(job_group).to have_been_destroyed
       end
     end
@@ -267,6 +279,17 @@ class OnCancellationJob
                                              on_cancellation_job_options: {})
       end
 
+      let(:error_reporter) { Proc.new { |_error| } }
+
+      around do |example|
+        original_error_reporter = Delayed::JobGroups.configuration.error_reporter
+        Delayed::JobGroups.configuration.error_reporter = error_reporter
+        example.run
+        Delayed::JobGroups.configuration.error_reporter = original_error_reporter
+      end
+
+      before { allow(error_reporter).to receive(:call) }
+
       it "handles missing on_cancellation_job" do
         job_group = Delayed::JobGroups::JobGroup.create!(on_cancellation_job: Delayed::JobGroups::JobGroupTestHelper::OnCancellationJob.new,
                                                          on_cancellation_job_options: {})
@@ -278,6 +301,7 @@ class OnCancellationJob
 
         # Deserialization fails
         expect { job_group.cancel }.not_to raise_error
+        expect(error_reporter).to have_received(:call)
         expect(job_group).to have_been_destroyed
       end
     end

From 630af067cf8dfb5c56d9de5c6ddf4f7dbb2d79d2 Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Tue, 16 Nov 2021 10:07:00 -0500
Subject: [PATCH 3/9] Fix some rubocop failures

---
 lib/delayed/job_groups/configuration.rb | 1 -
 lib/delayed/job_groups/job_group.rb     | 6 ++++--
 lib/delayed_job_groups_plugin.rb        | 6 ++----
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/delayed/job_groups/configuration.rb b/lib/delayed/job_groups/configuration.rb
index af38ebb..3884aaf 100644
--- a/lib/delayed/job_groups/configuration.rb
+++ b/lib/delayed/job_groups/configuration.rb
@@ -10,4 +10,3 @@ class Configuration
     end
   end
 end
-
diff --git a/lib/delayed/job_groups/job_group.rb b/lib/delayed/job_groups/job_group.rb
index 6171789..ff62adc 100644
--- a/lib/delayed/job_groups/job_group.rb
+++ b/lib/delayed/job_groups/job_group.rb
@@ -56,7 +56,8 @@ def cancel
           job = on_cancellation_job
           job_options = on_cancellation_job_options
         rescue StandardError => e
-          Delayed::Worker.logger.info("Failed to deserialize the on_cancellation_job or on_cancellation_job_options for job_group_id=#{id}. Skipping on_cancellation_job to clean up job group.")
+          Delayed::Worker.logger.info('Failed to deserialize the on_cancellation_job or on_cancellation_job_options ' \
+                                      "for job_group_id=#{id}. Skipping on_cancellation_job to clean up job group.")
           error_reporter.call(e) if error_reporter
         end
 
@@ -101,7 +102,8 @@ def complete
           job = on_completion_job
           job_options = on_completion_job_options
         rescue StandardError => e
-          Delayed::Worker.logger.info("Failed to deserialize the on_completion_job or on_completion_job_options for job_group_id=#{id}. Skipping on_completion_job to clean up job group.")
+          Delayed::Worker.logger.info('Failed to deserialize the on_completion_job or on_completion_job_options for ' \
+                                      "job_group_id=#{id}. Skipping on_completion_job to clean up job group.")
           error_reporter.call(e) if error_reporter
         end
 
diff --git a/lib/delayed_job_groups_plugin.rb b/lib/delayed_job_groups_plugin.rb
index 4077d01..0d20d31 100644
--- a/lib/delayed_job_groups_plugin.rb
+++ b/lib/delayed_job_groups_plugin.rb
@@ -27,13 +27,11 @@ module JobGroups
     @configuration = Delayed::JobGroups::Configuration.new
 
     class << self
+      attr_reader :configuration
+
       def configure
         yield(configuration) if block_given?
       end
-
-      def configuration
-        @configuration
-      end
     end
   end
 end

From 593cdfa493bfd29e887f0e1ef9a0d6860a01db66 Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Tue, 16 Nov 2021 10:16:34 -0500
Subject: [PATCH 4/9] Fix rubocop

---
 spec/delayed/job_groups/job_group_spec.rb | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/spec/delayed/job_groups/job_group_spec.rb b/spec/delayed/job_groups/job_group_spec.rb
index 741acbe..1af3b15 100644
--- a/spec/delayed/job_groups/job_group_spec.rb
+++ b/spec/delayed/job_groups/job_group_spec.rb
@@ -142,11 +142,14 @@
     end
 
     context "on_completion_job refers to missing class" do
+      # The on_completion_job needs the class to be defined this way in order to serialize it
+      # rubocop:disable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
       module Delayed::JobGroups::JobGroupTestHelper
         class OnCompletionJob
 
         end
       end
+      # rubocop:enable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
 
       let(:error_reporter) { Proc.new { |_error| } }
 
@@ -160,7 +163,8 @@ class OnCompletionJob
       before { allow(error_reporter).to receive(:call) }
 
       it "handles missing on_completion_job" do
-        job_group = Delayed::JobGroups::JobGroup.create!(on_completion_job: Delayed::JobGroups::JobGroupTestHelper::OnCompletionJob.new,
+        on_completion_job = Delayed::JobGroups::JobGroupTestHelper::OnCompletionJob.new
+        job_group = Delayed::JobGroups::JobGroup.create!(on_completion_job: on_completion_job,
                                                          on_completion_job_options: {})
         job = Delayed::Job.create!(job_group_id: job_group.id)
         job_group.mark_queueing_complete
@@ -268,16 +272,14 @@ class OnCompletionJob
     end
 
     context "on_cancellation_job refers to missing class" do
+      # The on_cancellation_job needs the class to be defined this way in order to serialize it
+      # rubocop:disable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
       module Delayed::JobGroups::JobGroupTestHelper
         class OnCancellationJob
 
         end
       end
-
-      subject(:job_group) do
-        Delayed::JobGroups::JobGroup.create!(on_cancellation_job: Delayed::JobGroups::JobGroupTestHelper::OnCancellationJob.new,
-                                             on_cancellation_job_options: {})
-      end
+      # rubocop:enable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
 
       let(:error_reporter) { Proc.new { |_error| } }
 
@@ -291,7 +293,8 @@ class OnCancellationJob
       before { allow(error_reporter).to receive(:call) }
 
       it "handles missing on_cancellation_job" do
-        job_group = Delayed::JobGroups::JobGroup.create!(on_cancellation_job: Delayed::JobGroups::JobGroupTestHelper::OnCancellationJob.new,
+        on_cancellation_job = Delayed::JobGroups::JobGroupTestHelper::OnCancellationJob.new
+        job_group = Delayed::JobGroups::JobGroup.create!(on_cancellation_job: on_cancellation_job,
                                                          on_cancellation_job_options: {})
 
         # Remove the class for on_cancellation_job

From a4dd7506ee2275f15fbc6775e142cce62fcc1284 Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Tue, 16 Nov 2021 10:19:12 -0500
Subject: [PATCH 5/9] Clean up

---
 lib/delayed/job_groups/configuration.rb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/delayed/job_groups/configuration.rb b/lib/delayed/job_groups/configuration.rb
index 3884aaf..bfbc0ee 100644
--- a/lib/delayed/job_groups/configuration.rb
+++ b/lib/delayed/job_groups/configuration.rb
@@ -1,8 +1,5 @@
 # frozen_string_literal: true
 
-require 'delayed_job'
-require 'set'
-
 module Delayed
   module JobGroups
     class Configuration

From fd110034d3e698f23f9f7564ebf73cd0e6729a99 Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Tue, 16 Nov 2021 10:28:08 -0500
Subject: [PATCH 6/9] Add changelog & version bump

---
 CHANGELOG.md                      | 3 +++
 lib/delayed/job_groups/version.rb | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2fd1e42..5e8693a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,7 @@
 # Changelog
+## 0.8.0
+* Failure to deserialize on_cancellation_job or on_completion_job will not prevent clean up of the job group.
+
 ### 0.7.0
 * Add support for ruby 3
 * Drop support for ruby < 2.6
diff --git a/lib/delayed/job_groups/version.rb b/lib/delayed/job_groups/version.rb
index f1ac4b7..6df91e4 100644
--- a/lib/delayed/job_groups/version.rb
+++ b/lib/delayed/job_groups/version.rb
@@ -2,6 +2,6 @@
 
 module Delayed
   module JobGroups
-    VERSION = '0.7.0'
+    VERSION = '0.8.0'
   end
 end

From 08bd18d9bc57420c3e5e7432b4f8151926f88073 Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Wed, 17 Nov 2021 15:22:49 -0500
Subject: [PATCH 7/9] PR Feedback * Explicit error classes rescued * Test begin
 block * Add failed_at column to delayed_job_groups

---
 CHANGELOG.md                                  |  3 +++
 lib/delayed/job_groups/job_group.rb           | 24 ++++++++++++-------
 ...iled_at_to_delayed_job_groups_generator.rb | 22 +++++++++++++++++
 .../add_failed_at_to_delayed_job_groups.erb   | 12 ++++++++++
 .../templates/migration.erb                   |  1 +
 spec/db/schema.rb                             |  1 +
 spec/delayed/job_groups/job_group_spec.rb     | 18 +++++++++-----
 7 files changed, 66 insertions(+), 15 deletions(-)
 create mode 100644 lib/generators/delayed_job_groups_plugin/add_failed_at_to_delayed_job_groups_generator.rb
 create mode 100644 lib/generators/delayed_job_groups_plugin/templates/add_failed_at_to_delayed_job_groups.erb

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5e8693a..10e8d98 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,9 @@
 # Changelog
 ## 0.8.0
 * Failure to deserialize on_cancellation_job or on_completion_job will not prevent clean up of the job group.
+* Adds `failed_at` to `delayed_job_groups` table. 
+Use `bundle exec rails g delayed_job_groups_plugin:add_failed_at_to_delayed_job_groups` to generate the migration to
+add this column.
 
 ### 0.7.0
 * Add support for ruby 3
diff --git a/lib/delayed/job_groups/job_group.rb b/lib/delayed/job_groups/job_group.rb
index ff62adc..205f9d5 100644
--- a/lib/delayed/job_groups/job_group.rb
+++ b/lib/delayed/job_groups/job_group.rb
@@ -55,15 +55,20 @@ def cancel
         begin
           job = on_cancellation_job
           job_options = on_cancellation_job_options
-        rescue StandardError => e
+
+          self.class.transaction do
+            Delayed::Job.enqueue(job, job_options || {}) if job
+            destroy
+          end
+        rescue TypeError, LoadError, NameError, ArgumentError, SyntaxError, Psych::SyntaxError => e
           Delayed::Worker.logger.info('Failed to deserialize the on_cancellation_job or on_cancellation_job_options ' \
                                       "for job_group_id=#{id}. Skipping on_cancellation_job to clean up job group.")
           error_reporter.call(e) if error_reporter
+          self.class.transaction do
+            update_columns(failed_at: Time.now)
+            queued_jobs.delete_all
+          end
         end
-
-        Delayed::Job.enqueue(job, job_options || {}) if job
-
-        destroy
       end
 
       def self.check_for_completion(job_group_id)
@@ -101,14 +106,15 @@ def complete
         begin
           job = on_completion_job
           job_options = on_completion_job_options
-        rescue StandardError => e
+
+          Delayed::Job.enqueue(job, job_options || {}) if job
+          destroy
+        rescue TypeError, LoadError, NameError, ArgumentError, SyntaxError, Psych::SyntaxError => e
           Delayed::Worker.logger.info('Failed to deserialize the on_completion_job or on_completion_job_options for ' \
                                       "job_group_id=#{id}. Skipping on_completion_job to clean up job group.")
           error_reporter.call(e) if error_reporter
+          update_columns(failed_at: Time.now)
         end
-
-        Delayed::Job.enqueue(job, job_options || {}) if job
-        destroy
       end
 
       def error_reporter
diff --git a/lib/generators/delayed_job_groups_plugin/add_failed_at_to_delayed_job_groups_generator.rb b/lib/generators/delayed_job_groups_plugin/add_failed_at_to_delayed_job_groups_generator.rb
new file mode 100644
index 0000000..75b4cf6
--- /dev/null
+++ b/lib/generators/delayed_job_groups_plugin/add_failed_at_to_delayed_job_groups_generator.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require 'rails/generators'
+require 'rails/generators/migration'
+require 'rails/generators/active_record'
+
+module DelayedJobGroupsPlugin
+  class AddFailedAtToDelayedJobGroupsGenerator < Rails::Generators::Base
+    include Rails::Generators::Migration
+
+    source_paths << File.join(File.dirname(__FILE__), 'templates')
+
+    def create_migration_file
+      migration_template('add_failed_at_to_delayed_job_groups.erb', 'db/migrate/add_failed_at_to_delayed_job_groups.rb')
+    end
+
+    def self.next_migration_number(dirname)
+      ActiveRecord::Generators::Base.next_migration_number(dirname)
+    end
+
+  end
+end
diff --git a/lib/generators/delayed_job_groups_plugin/templates/add_failed_at_to_delayed_job_groups.erb b/lib/generators/delayed_job_groups_plugin/templates/add_failed_at_to_delayed_job_groups.erb
new file mode 100644
index 0000000..9bbe89f
--- /dev/null
+++ b/lib/generators/delayed_job_groups_plugin/templates/add_failed_at_to_delayed_job_groups.erb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class AddFailedAtToDelayedJobGroups < ActiveRecord::Migration[<%= ActiveRecord::VERSION::MAJOR %>.<%= ActiveRecord::VERSION::MINOR %>]
+
+  def up
+    add_column(:delayed_job_groups, :failed_at, :timestamp)
+  end
+
+  def down
+    remove_column(:delayed_job_groups, :failed_at)
+  end
+end
diff --git a/lib/generators/delayed_job_groups_plugin/templates/migration.erb b/lib/generators/delayed_job_groups_plugin/templates/migration.erb
index baf5c91..b3dc0d0 100644
--- a/lib/generators/delayed_job_groups_plugin/templates/migration.erb
+++ b/lib/generators/delayed_job_groups_plugin/templates/migration.erb
@@ -24,6 +24,7 @@ class CreateDelayedJobGroups < ActiveRecord::Migration[<%= ActiveRecord::VERSION
       t.boolean :failure_cancels_group, default: true, null: false
       t.boolean :queueing_complete, default: false, null: false
       t.boolean :blocked, default: false, null: false
+      t.timestamp :failed_at
     end
   end
 
diff --git a/spec/db/schema.rb b/spec/db/schema.rb
index aea1948..0ea7d78 100644
--- a/spec/db/schema.rb
+++ b/spec/db/schema.rb
@@ -26,5 +26,6 @@
     t.boolean :failure_cancels_group, default: true, null: false
     t.boolean :queueing_complete, default: false, null: false
     t.boolean :blocked, default: false, null: false
+    t.timestamp :failed_at
   end
 end
diff --git a/spec/delayed/job_groups/job_group_spec.rb b/spec/delayed/job_groups/job_group_spec.rb
index 1af3b15..e1465f7 100644
--- a/spec/delayed/job_groups/job_group_spec.rb
+++ b/spec/delayed/job_groups/job_group_spec.rb
@@ -144,9 +144,11 @@
     context "on_completion_job refers to missing class" do
       # The on_completion_job needs the class to be defined this way in order to serialize it
       # rubocop:disable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
-      module Delayed::JobGroups::JobGroupTestHelper
-        class OnCompletionJob
+      before do
+        module Delayed::JobGroups::JobGroupTestHelper
+          class OnCompletionJob
 
+          end
         end
       end
       # rubocop:enable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
@@ -178,7 +180,8 @@ class OnCompletionJob
         # Deserialization fails
         expect { Delayed::JobGroups::JobGroup.check_for_completion(job_group.id) }.not_to raise_error
         expect(error_reporter).to have_received(:call)
-        expect(job_group).to have_been_destroyed
+        expect(job_group).not_to have_been_destroyed
+        expect(job_group.reload.failed_at).to be_present
       end
     end
   end
@@ -274,9 +277,11 @@ class OnCompletionJob
     context "on_cancellation_job refers to missing class" do
       # The on_cancellation_job needs the class to be defined this way in order to serialize it
       # rubocop:disable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
-      module Delayed::JobGroups::JobGroupTestHelper
-        class OnCancellationJob
+      before do
+        module Delayed::JobGroups::JobGroupTestHelper
+          class OnCancellationJob
 
+          end
         end
       end
       # rubocop:enable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
@@ -305,7 +310,8 @@ class OnCancellationJob
         # Deserialization fails
         expect { job_group.cancel }.not_to raise_error
         expect(error_reporter).to have_received(:call)
-        expect(job_group).to have_been_destroyed
+        expect(job_group).not_to have_been_destroyed
+        expect(job_group.reload.failed_at).to be_present
       end
     end
   end

From 26f58af2e2b30d2230c9bf5a86388268c88912cf Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Wed, 17 Nov 2021 15:37:55 -0500
Subject: [PATCH 8/9] Clean up

---
 lib/delayed/job_groups/job_group.rb | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/lib/delayed/job_groups/job_group.rb b/lib/delayed/job_groups/job_group.rb
index 205f9d5..3a62375 100644
--- a/lib/delayed/job_groups/job_group.rb
+++ b/lib/delayed/job_groups/job_group.rb
@@ -48,16 +48,10 @@ def unblock
       end
 
       def cancel
-        job = nil
-        job_options = nil
-
-        # Deserialization of the job or its options can fail
         begin
-          job = on_cancellation_job
-          job_options = on_cancellation_job_options
-
           self.class.transaction do
-            Delayed::Job.enqueue(job, job_options || {}) if job
+            # Deserialization of the job or its options can fail
+            Delayed::Job.enqueue(on_cancellation_job, on_cancellation_job_options || {}) if on_cancellation_job
             destroy
           end
         rescue TypeError, LoadError, NameError, ArgumentError, SyntaxError, Psych::SyntaxError => e
@@ -99,15 +93,9 @@ def ready_for_completion?
       end
 
       def complete
-        job = nil
-        job_options = nil
-
-        # Deserialization of the job or its options can fail
         begin
-          job = on_completion_job
-          job_options = on_completion_job_options
-
-          Delayed::Job.enqueue(job, job_options || {}) if job
+          # Deserialization of the job or its options can fail
+          Delayed::Job.enqueue(on_completion_job, on_completion_job_options || {}) if on_completion_job
           destroy
         rescue TypeError, LoadError, NameError, ArgumentError, SyntaxError, Psych::SyntaxError => e
           Delayed::Worker.logger.info('Failed to deserialize the on_completion_job or on_completion_job_options for ' \

From 8903a2b81a30768f437a43756a693a7f29930c78 Mon Sep 17 00:00:00 2001
From: Will Leonard <wleonard@salsify.com>
Date: Wed, 17 Nov 2021 16:26:22 -0500
Subject: [PATCH 9/9] Fix rubocop

---
 lib/delayed/job_groups/job_group.rb       | 44 +++++++++++------------
 spec/delayed/job_groups/job_group_spec.rb | 15 ++++----
 2 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/lib/delayed/job_groups/job_group.rb b/lib/delayed/job_groups/job_group.rb
index 3a62375..f1ef3db 100644
--- a/lib/delayed/job_groups/job_group.rb
+++ b/lib/delayed/job_groups/job_group.rb
@@ -48,20 +48,18 @@ def unblock
       end
 
       def cancel
-        begin
-          self.class.transaction do
-            # Deserialization of the job or its options can fail
-            Delayed::Job.enqueue(on_cancellation_job, on_cancellation_job_options || {}) if on_cancellation_job
-            destroy
-          end
-        rescue TypeError, LoadError, NameError, ArgumentError, SyntaxError, Psych::SyntaxError => e
-          Delayed::Worker.logger.info('Failed to deserialize the on_cancellation_job or on_cancellation_job_options ' \
-                                      "for job_group_id=#{id}. Skipping on_cancellation_job to clean up job group.")
-          error_reporter.call(e) if error_reporter
-          self.class.transaction do
-            update_columns(failed_at: Time.now)
-            queued_jobs.delete_all
-          end
+        self.class.transaction do
+          # Deserialization of the job or its options can fail
+          Delayed::Job.enqueue(on_cancellation_job, on_cancellation_job_options || {}) if on_cancellation_job
+          destroy
+        end
+      rescue TypeError, LoadError, NameError, ArgumentError, SyntaxError, Psych::SyntaxError => e
+        Delayed::Worker.logger.info('Failed to deserialize the on_cancellation_job or on_cancellation_job_options ' \
+                                    "for job_group_id=#{id}. Skipping on_cancellation_job to clean up job group.")
+        error_reporter.call(e) if error_reporter
+        self.class.transaction do
+          update_columns(failed_at: Time.now)
+          queued_jobs.delete_all
         end
       end
 
@@ -93,16 +91,14 @@ def ready_for_completion?
       end
 
       def complete
-        begin
-          # Deserialization of the job or its options can fail
-          Delayed::Job.enqueue(on_completion_job, on_completion_job_options || {}) if on_completion_job
-          destroy
-        rescue TypeError, LoadError, NameError, ArgumentError, SyntaxError, Psych::SyntaxError => e
-          Delayed::Worker.logger.info('Failed to deserialize the on_completion_job or on_completion_job_options for ' \
-                                      "job_group_id=#{id}. Skipping on_completion_job to clean up job group.")
-          error_reporter.call(e) if error_reporter
-          update_columns(failed_at: Time.now)
-        end
+        # Deserialization of the job or its options can fail
+        Delayed::Job.enqueue(on_completion_job, on_completion_job_options || {}) if on_completion_job
+        destroy
+      rescue TypeError, LoadError, NameError, ArgumentError, SyntaxError, Psych::SyntaxError => e
+        Delayed::Worker.logger.info('Failed to deserialize the on_completion_job or on_completion_job_options for ' \
+                                    "job_group_id=#{id}. Skipping on_completion_job to clean up job group.")
+        error_reporter.call(e) if error_reporter
+        update_columns(failed_at: Time.now)
       end
 
       def error_reporter
diff --git a/spec/delayed/job_groups/job_group_spec.rb b/spec/delayed/job_groups/job_group_spec.rb
index e1465f7..dfa0303 100644
--- a/spec/delayed/job_groups/job_group_spec.rb
+++ b/spec/delayed/job_groups/job_group_spec.rb
@@ -142,6 +142,8 @@
     end
 
     context "on_completion_job refers to missing class" do
+      let(:error_reporter) { Proc.new { |_error| } }
+
       # The on_completion_job needs the class to be defined this way in order to serialize it
       # rubocop:disable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
       before do
@@ -150,10 +152,11 @@ class OnCompletionJob
 
           end
         end
+
+        allow(error_reporter).to receive(:call)
       end
       # rubocop:enable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
 
-      let(:error_reporter) { Proc.new { |_error| } }
 
       around do |example|
         original_error_reporter = Delayed::JobGroups.configuration.error_reporter
@@ -162,8 +165,6 @@ class OnCompletionJob
         Delayed::JobGroups.configuration.error_reporter = original_error_reporter
       end
 
-      before { allow(error_reporter).to receive(:call) }
-
       it "handles missing on_completion_job" do
         on_completion_job = Delayed::JobGroups::JobGroupTestHelper::OnCompletionJob.new
         job_group = Delayed::JobGroups::JobGroup.create!(on_completion_job: on_completion_job,
@@ -275,6 +276,8 @@ class OnCompletionJob
     end
 
     context "on_cancellation_job refers to missing class" do
+      let(:error_reporter) { Proc.new { |_error| } }
+
       # The on_cancellation_job needs the class to be defined this way in order to serialize it
       # rubocop:disable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
       before do
@@ -283,11 +286,11 @@ class OnCancellationJob
 
           end
         end
+
+        allow(error_reporter).to receive(:call)
       end
       # rubocop:enable RSpec/LeakyConstantDeclaration,Style/ClassAndModuleChildren,Lint/ConstantDefinitionInBlock
 
-      let(:error_reporter) { Proc.new { |_error| } }
-
       around do |example|
         original_error_reporter = Delayed::JobGroups.configuration.error_reporter
         Delayed::JobGroups.configuration.error_reporter = error_reporter
@@ -295,8 +298,6 @@ class OnCancellationJob
         Delayed::JobGroups.configuration.error_reporter = original_error_reporter
       end
 
-      before { allow(error_reporter).to receive(:call) }
-
       it "handles missing on_cancellation_job" do
         on_cancellation_job = Delayed::JobGroups::JobGroupTestHelper::OnCancellationJob.new
         job_group = Delayed::JobGroups::JobGroup.create!(on_cancellation_job: on_cancellation_job,