sunng87 / diehard

Clojure resilience library for flexible retry, circuit breaker and rate limiter
Eclipse Public License 2.0
331 stars 27 forks source link

NullPointerException when combining with-retry and with-timeout #56

Closed msolli closed 1 year ago

msolli commented 1 year ago

I want to retry something that might time out, an HTTP request maybe. So I tried this:

(diehard/with-retry {:retry-on    TimeoutExceededException
                     :max-retries 1}
  (diehard/with-timeout {:timeout-ms 100}
    ;; Something that might time out
    (Thread/sleep 200)))

This results in a NullPointerException:

Execution error (NullPointerException) at user/eval173014 (user.clj:301).
Cannot throw exception because the return value of "java.lang.Throwable.getCause()" is null

My guess is it happens because the throwed TimeoutExceededException does not have a cause:

(try
    (diehard/with-timeout {:timeout-ms 100}
      (Thread/sleep 200))
    (catch Exception e
      (.getCause e)))
=> nil

Yet with-retry assumes that a FailsafeException always has a cause: https://github.com/sunng87/diehard/blob/master/src/diehard/core.clj#L365

I've worked around this by throwing a new exception that has the TimeoutExceededException as cause:

(diehard/with-retry {:retry-on    Exception
                     :max-retries 1}
  (try
    (diehard/with-timeout {:timeout-ms 100}
      (Thread/sleep 200))
    (catch TimeoutExceededException e
      (throw (Exception. "Timeout" e)))))

But I'm wondering if there's a better intended way to do this, or if there maybe should be a check around the .getCause in with-retry that would throw the original exception if the cause was nil?

sunng87 commented 1 year ago

Thank you! As you said a check is needed here. I will create a fix for this.