sorentwo / oban

💎 Robust job processing in Elixir, backed by modern PostgreSQL and SQLite3
https://getoban.pro
Apache License 2.0
3.18k stars 297 forks source link

Oban.Pro.Workers.Chain with test inline mode causes Ecto error on comparison with nil id #1064

Closed kzemek closed 3 months ago

kzemek commented 3 months ago

Environment

Current Behavior

When config :app, Oban, testing: :inline is set, Oban.insert(job) fails for a use Oban.Pro.Workers.Chain worker with:

     ** (ArgumentError) comparison with nil is forbidden as it is unsafe. If you want to check if a value is nil, use is_nil/1 instead

     stacktrace:
       (ecto 3.11.2) lib/ecto/query/builder.ex:1069: Ecto.Query.Builder.not_nil!/1
       (oban_pro 1.4.2) lib/oban/pro/workers/chain.ex:252: Oban.Pro.Workers.Chain.maybe_process/3
       (app 0.1.0) lib/app/oban_worker.ex:11: App.ObanWorker.perform/1
       (oban 2.17.8) lib/oban/queue/executor.ex:129: Oban.Queue.Executor.perform/1
       (oban 2.17.8) lib/oban/queue/executor.ex:74: Oban.Queue.Executor.call/1
       (oban 2.17.8) lib/oban/engines/inline.ex:97: Oban.Engines.Inline.execute_job/2
       (oban 2.17.8) lib/oban/engines/inline.ex:37: Oban.Engines.Inline.insert_job/3
       (oban 2.17.8) lib/oban/engine.ex:181: anonymous fn/4 in Oban.Engine.insert_job/3
       (oban 2.17.8) lib/oban/engine.ex:319: anonymous fn/3 in Oban.Engine.with_span/4

Looks like the issue is that we don't include an explicit id in our jobs, expecting Oban to autogenerate them for us, but the autogeneration only happens on actual database insert and not with the inline engine.

Including id explicitly does work around the issue, so currently my prod code looks like this:

  if Mix.env() == :test do
    defp new_job(args) do
      %{changes: changes} = job = new(args)
      %{job | changes: Map.put(changes, :id, 1)}
    end
  else
    defp new_job(args), do: new(args)
  end

Expected Behavior

I'd expect the inline engine to not crash on Chain worker jobs without an explicit id.

sorentwo commented 3 months ago

@kzemek Thanks for the report. Fixed on main, it will be out in v1.4.4 this week.