refactor empty/timeout retry raising so that re-raised exceptions con…

…tain info as well
heroku · Jan 24, 2023 · 50d0168 · 50d0168
1 parent 207cf12
commit 50d0168
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 18 deletions.
diff --git a/lib/hatchet/heroku_run.rb b/lib/hatchet/heroku_run.rb
@@ -110,32 +110,30 @@ def call
       rescue HerokuRunEmptyOutputError => e
         if @retry_on_empty and @empty_fail_count < 3
           @empty_fail_count += 1
-          message = String.new("Empty output from run #{@dyno_id} with command #{@command}, retrying...")
-          message << "\nTo disable pass in `retry_on_empty: false` or set HATCHET_DISABLE_EMPTY_RUN_RETRY=1 globally"
-          message << "\nfailed_count: #{@empty_fail_count}"
-          message << "\nreleases: #{@app.releases}"
+          message = e.message
           message << "\n#{caller.join("\n")}"
+          message << "\nThis was failed attempt ##{@empty_fail_count}, now retrying execution."
+          message << "\nTo disable retries, pass in `retry_on_empty: false` or set HATCHET_DISABLE_EMPTY_RUN_RETRY=1 globally."
           @stderr.puts message
           sleep(@retry_delay) # without run_multi, this will prevent occasional "can only run one free dyno" errors
           retry
+        else
+          raise # we are out of retries
         end
-        raise # we are out of retries
       rescue HerokuRunTimeoutError => e
+        @app.platform_api.dyno.stop(@app.name, @dyno_id) if @dyno_id
         if @timeout_fail_count < 3
           @timeout_fail_count += 1
-          message = String.new("Run #{@dyno_id} with command #{@command} timed out after #{@timeout} seconds, stopping dyno and retrying...")
-          message << "\nstderr until moment of termination was: #{@result.stderr}"
-          message << "\nstdout until moment of termination was: #{@result.stdout}"
-          message << "\nTo disable pass in `timeout: 0` or set HATCHET_DEFAULT_RUN_TIMEOUT=0 globally"
-          message << "\nfailed_count: #{@timeout_fail_count}"
-          message << "\nreleases: #{@app.releases}"
+          message = e.message
           message << "\n#{caller.join("\n")}"
+          message << "\nThis was failed attempt ##{@timeout_fail_count}, now retrying execution."
+          message << "\nTo disable retries, pass in `timeout: 0` or set HATCHET_DEFAULT_RUN_TIMEOUT=0 globally."
           @stderr.puts message
-          @app.platform_api.dyno.stop(@app.name, @dyno_id) if @dyno_id
           sleep(@retry_delay) # without run_multi, this will prevent occasional "can only run one free dyno" errors
           retry
+        else
+          raise # we are out of retries
         end
-        raise # we are out of retries
       end
 
       self
@@ -145,8 +143,8 @@ def call
       ShellThrottle.new(platform_api: @app.platform_api).call do |throttle|
         run_shell!
         throw(:throttle) if @result.stderr.match?(/reached the API rate limit/)
-        raise HerokuRunTimeoutError if @result.status_obj.signaled? # program got terminated by our SIGTERM, raise
-        raise HerokuRunEmptyOutputError if @result.stdout.empty?
+        raise HerokuRunTimeoutError.new "Run #{@dyno_id} with command #{@command} timed out after #{@timeout} seconds, stopping dyno.\nstderr until moment of termination was:\n#{@result.stderr}\nstdout until moment of termination was: #{@result.stdout}\nReleases: #{@app.releases}" if @result.status_obj.signaled? # program got terminated by our SIGTERM, raise
+        raise HerokuRunEmptyOutputError.new "Empty output from run #{@dyno_id} with command #{@command}.\nReleases: #{@app.releases}" if @result.stdout.empty?
       end
     end
 

diff --git a/spec/unit/heroku_run_spec.rb b/spec/unit/heroku_run_spec.rb
@@ -59,7 +59,7 @@ def run_obj.run_shell!
       run_obj.call
 
       expect(run_obj.instance_variable_get(:@empty_fail_count)).to eq(3)
-      expect(stderr.string).to include("retrying...")
+      expect(stderr.string).to include("now retrying execution")
     end
 
     it "retries 0 times on NON empty result" do
@@ -93,7 +93,7 @@ def run_obj.run_shell!
       run_obj.call
 
       expect(run_obj.instance_variable_get(:@empty_fail_count)).to eq(0)
-      expect(stderr.string).to_not include("retrying...")
+      expect(stderr.string).to_not include("now retrying execution")
     end
 
     it "retries work when message is delivered via stderr" do
@@ -131,7 +131,7 @@ def run_obj.run_shell!
         run_obj.call
 
         expect(run_obj.instance_variable_get(:@empty_fail_count)).to eq(0)
-        expect(stderr.string).to_not include("retrying...")
+        expect(stderr.string).to_not include("now retrying execution")
       ensure
         ENV["HATCHET_DISABLE_EMPTY_RUN_RETRY"] = original_env
       end