ash icon indicating copy to clipboard operation
ash copied to clipboard

Handle Notifications when running an action inside an action

Open maennchen opened this issue 1 year ago • 1 comments

Describe the bug

When defining an action change, which internally calls another action using Ash.run_action, the following warning is logged and the notification is not sent.

     11:35:26.111 [warning] Missed 1 notifications in action [REDACTED].
     
     This happens when the resources are in a transaction, and you did not pass
     `return_notifications?: true`. If you are in a changeset hook, you can
     return the notifications. If not, you can send the notifications using
     `Ash.Notifier.notify/1` once your resources are out of a transaction.
     
         (elixir 1.17.3) lib/process.ex:864: Process.info/2
         (ash 3.4.21) lib/ash/actions/helpers.ex:349: Ash.Actions.Helpers.warn_missed!/3
         (ash 3.4.21) lib/ash/actions/action.ex:143: Ash.Actions.Action.run/3
         (ash 3.4.21) lib/ash.ex:1384: Ash.run_action/2
         (ash 3.4.21) lib/ash/actions/action.ex:129: Ash.Actions.Action.run/3
         (ash 3.4.21) lib/ash.ex:1384: Ash.run_action/2
         ([REDACTED] 0.1.0) lib/[REDACTED].ex:27: anonymous fn/2 in [REDACTED].change/3
         (ash 3.4.21) lib/ash/changeset/changeset.ex:3803: anonymous fn/2 in Ash.Changeset.run_after_actions/3
         (elixir 1.17.3) lib/enum.ex:4858: Enumerable.List.reduce/3
         (elixir 1.17.3) lib/enum.ex:2585: Enum.reduce_while/3
         (ash 3.4.21) lib/ash/changeset/changeset.ex:3280: anonymous fn/3 in Ash.Changeset.with_hooks/3
         (ecto_sql 3.12.0) lib/ecto/adapters/sql.ex:1382: anonymous fn/3 in Ecto.Adapters.SQL.checkout_or_transaction/4
         (db_connection 2.7.0) lib/db_connection.ex:1756: DBConnection.run_transaction/4
         (ash 3.4.21) lib/ash/changeset/changeset.ex:3278: anonymous fn/3 in Ash.Changeset.with_hooks/3
         (ash 3.4.21) lib/ash/changeset/changeset.ex:3422: anonymous fn/2 in Ash.Changeset.transaction_hooks/2
         (ash 3.4.21) lib/ash/changeset/changeset.ex:3259: Ash.Changeset.with_hooks/3
         (ash 3.4.21) lib/ash/actions/destroy/destroy.ex:166: Ash.Actions.Destroy.commit/3
         (ash 3.4.21) lib/ash/actions/destroy/destroy.ex:108: Ash.Actions.Destroy.do_run/4
         (ash 3.4.21) lib/ash/actions/destroy/destroy.ex:57: Ash.Actions.Destroy.run/4
         (ash 3.4.21) lib/ash.ex:2743: Ash.destroy/2
     
     
     While you should likely leave this setting on, you can ignore these or turn them into errors.
     
     To ignore these in all cases:
     
     config :ash, :missed_notifications, :ignore
     
     To turn this into raised errors:
     
     config :ash, :missed_notifications, :raise

To Reproduce

  • Define change on resource action.
  • In change, call Ash.Changeset.after_action/2
  • Inside of the after action function, call Ash.run_action for any other action

Expected behavior

The logged warning mentions the option return_notifications?: true. This option is not valid for Ash.run_action.

Also there should be a way to retrieve the notifications from Ash.run_action instead of already sending them and return them in the Ash.Changeset.after_action/2 result. For example by actually implementing the return_notifications?: true option.

Runtime

  • Elixir version: 1.17.3
  • Erlang version: 27.1
  • OS: Ubuntu 24.04
  • Ash version: 3.4.21
  • any related extension versions:
* ash_admin 0.11.6 (Hex package) (mix)
  locked at 0.11.6 (ash_admin) 6419207e
* ash_graphql 1.3.4 (Hex package) (mix)
  locked at 1.3.4 (ash_graphql) 52c803ef
* ash_json_api 1.4.8 (Hex package) (mix)
  locked at 1.4.8 (ash_json_api) ee244cb8
* ash_oban 0.2.5 (Hex package) (mix)
  locked at 0.2.5 (ash_oban) b0451cfc
* ash_phoenix 2.1.2 (Hex package) (mix)
  locked at 2.1.2 (ash_phoenix) b591bd73
* ash_postgres 2.4.2 (Hex package) (mix)
  locked at 2.4.2 (ash_postgres) fb3f14fc
* ash_sql 0.2.32 (Hex package) (mix)
  locked at 0.2.32 (ash_sql) 43773bcd

Additional context

  • Valid Options for Ash.run_action - https://github.com/ash-project/ash/blob/c4c8dbf79f193c5f0d5ab79e28c71c42f4833eb8/lib/ash.ex#L721-L755
  • Invalid Option Check: https://github.com/ash-project/ash/blob/c4c8dbf79f193c5f0d5ab79e28c71c42f4833eb8/lib/ash/actions/action.ex#L101

maennchen avatar Oct 04 '24 12:10 maennchen

Right, so the behavior that needs to be fixed here is a couple things.

  1. as you mentioned, return_notifications? option must be added & honored.
  2. additionally, when you are already in a transaction, the other action types aggregate notifications in the process dictionary, eliminating the need for return_notifications?: true in most cases.

zachdaniel avatar Oct 04 '24 12:10 zachdaniel