ash-project
misleading error if arguments are forgotten in Ash.Query.for_read
Describe the bug
When building a read query for an action with only optional arguments, a misleading error message occurs when no arguments are passed.
iex(1)> Ash.Query.for_read(Hsm.FFL.ADRecord, :bound_book_report, actor: nil)
** (Ash.Error.Forbidden) Forbidden Error
* The domain Hsm.FFL requires that an actor is provided at all times and none was provided.
(elixir 1.17.3) lib/process.ex:864: Process.info/2
(ash 3.4.35)
To Reproduce
Build a read query on any action that has only optional arguments. Not sure if the same error also happens when the action has mandatory arguments.
Expected behavior
The error should indicate that it is an action that accepts arguments, but no arguments were passed. That or maybe it should automatically insert an empty map as the arguments? :thinking:
We should probably validate inputs first? Or require that inputs is a map? That second one would be a breaking change. Anyway, definitely confusing.
I couldn't reproduce this. Simply running Ash.Query.for_read(resource, :action) returns an appropriate error:
iex(3)> Ash.Query.for_read(Ash.Test.QueryTest.User, :by_id)
#Ash.Query<
resource: Ash.Test.QueryTest.User,
filter: #Ash.Filter<id == nil>,
errors: [
%Ash.Error.Query.Required{
field: :id,
type: :argument,
resource: Ash.Test.QueryTest.User,
splode: Ash.Error,
bread_crumbs: [],
vars: [],
path: [],
stacktrace: #Splode.Stacktrace<>,
class: :invalid
}
]
>
Could it be that your domain force authorization, @frankdugan3? Because in that case the error looks exactly like you described (and it makes sense to me):
iex(5)> Ash.Query.for_read(Ash.Test.QueryTest.User, :by_id)
** (Ash.Error.Forbidden) Forbidden Error
* The domain Ash.Test.Domain requires that an actor is provided at all times and none was provided.
If a keyword list is passed then it should be treated as an opts list, so it should see actor: nil. the check to require an actor allows nil as a value, it just requires specifying it in some way.
@frankdugan3 If you do Ash.Query.for_read(Hsm.FFL.ADRecord, :bound_book_report, %{}, actor: nil) should be fine.
I think the problem here is the last two args that have default, map and list. In your case actor: nil is args, not opts.
@zachdaniel Something like:
def for_read(query, action_name, opts) when is_list(opts) do
for_read(query, action_name, %{}, opts)
end
could catch this case. wdyt?
@frankdugan3 If you do
Ash.Query.for_read(Hsm.FFL.ADRecord, :bound_book_report, %{}, actor: nil)should be fine.
To clarify, I understand how to work around this, the issue is about the error being misleading.
That aside, I think your solution of the guard clause on for_read is the correct fix.
@frankdugan3 If you do
Ash.Query.for_read(Hsm.FFL.ADRecord, :bound_book_report, %{}, actor: nil)should be fine.To clarify, I understand how to work around this, the issue is about the error being misleading.
That aside, I think your solution of the guard clause on
for_readis the correct fix.
Yea, but it's a breaking change, so for version up.
Unfortunately there's nothing we can do about this. In 4.0 we will make the inputs required for the lower level interfaces like this to solve this issue.