cambiatus / backend

Cambiatus GraphQL API
GNU Affero General Public License v3.0
21 stars 18 forks source link

[BUG] minor issue with error handling in two Oban jobs #304

Closed seth-schroeder closed 1 year ago

seth-schroeder commented 1 year ago

Story

Hello! I was testing a custom Credo check and it reported a warning with two Oban workers in this project:

seth-schroeder@ubuntu:~/src/elixir/backend$ mix credo
Checking 180 source files (this might take a while) ...

  Warnings - please take a look
┃
┃ [W] → Potential false negative on Multi error handling in an Oban job
┃       lib/cambiatus/workers/monthly_digest_worker.ex:24 #(Cambiatus.Workers.MonthlyDigestWorker.perform)
┃ [W] → Potential false negative on Multi error handling in an Oban job
┃       lib/cambiatus/workers/contribution_paypal_worker.ex:38 #(Cambiatus.Workers.ContributionPaypalWorker.digest_payment_callback)

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 0.9 seconds (0.1s to load, 0.7s running 53 checks on 180 files)
847 mods/funs, found 2 warnings.

The warning has to do with returning a 4 tuple error from perform. Oban accepts only 2 tuple errors. When Multi.new is part of a Repo.transaction, 4 tuple errors are generated. The 4 tuple errors are interpreted as successes by Oban.

If you are interested, there are more details in the links section.

I was going to submit two PRs for consideration:

Links

https://github.com/seth-schroeder/credo_check_error_handling_ecto_oban#credocheckerrorhandlingectooban https://hex.pm/packages/credo_check_error_handling_ecto_oban