Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This should not be possible, please report a detailed bug at error when error is very clear #1606

Open
jeroen11dijk opened this issue Nov 21, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@jeroen11dijk
Copy link

Bug Report

Describe the Bug

When attempting to use Ash.Sort.expr_sort with a resource or relationship that does not exist, the error message suggests reporting a bug instead of providing a clear and direct error message about the invalid resource/relationship.


To Reproduce

Add the following code to a resource:

prepare build(
  sort: [
    Ash.Sort.expr_sort(not_existing_resource_or_relationship.attribute, :string)
  ]
)

And you get the error

** (Ash.Error.Framework.AssumptionFailed) Assumption failed: Listing refs in not_existing_resource_or_relationship.attribute:

Found reference :attribute at path [:not_existing_resource_or_relationship]

No related resource at path [:not_existing_resource_or_relationship] from Cvs.MatchingTable.Entry

This should not be possible, please report a detailed bug at:

https://github.com/ash-project/ash/issues/new?assignees=&labels=bug%2C+needs+review&template=bug_report.md&title=

Expected Behavior

A more specific error message pointing directly to the Ash.Sort.expr_sort call and that it is a human error. Not asying that you should report a bug here.

Runtime

[ - Elixir version](elixir: "> 1.16",)
{:ash, "> 3.4.37"},
{:ash_postgres, "> 2.4.12"},
{:ash_phoenix, "> 2.0"},
{:ash_admin, "> 0.11.9"},
{:ash_authentication, "> 4.2.7"},
{:ash_authentication_phoenix, "~> 2.1.2"},

@ken-kost
Copy link
Contributor

@zachdaniel Is this fixed? Seems like it is.
When I try this out compile doesn't pass:

    error: undefined variable "not_existing_resource_or_relationship"
    │
 29 │                   Ash.Sort.expr_sort(not_existing_resource_or_relationship.attribute, :string)
    │                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    │

== Compilation error in file lib/nutrea/food.ex ==
** (CompileError) cannot compile module (errors have been logged)
    (ash 3.4.64) expanding macro: Ash.Resource.Dsl.Actions.Read.Actions.Preparations.Prepare.prepare/1

@zachdaniel
Copy link
Contributor

You have to first require Ash.Sort

@ken-kost
Copy link
Contributor

I didn't reproduce the same error but something similar:

Unknown Error

* ** (MatchError) no match of right hand side value: {:error, "Invalid reference not_existing_resource_or_relationship.attribute"}
  (ash 3.4.64) lib/ash/actions/read/read.ex:1833: anonymous fn/8 in Ash.Actions.Read.add_calc_context_to_sort/8

which happens on:

            {:ok, expr} =            # <- L1833
              Ash.Filter.hydrate_refs(
                expr,
                %{
                  resource: resource,
                  public?: false,
                  parent_stack: opts[:parent_stack]
                }
              )

This happens in add_calc_context_to_sort which is called in do_read

         query <- %{
           query
           | ...,
             sort:
               add_calc_context_to_sort(
                 query,
                 opts[:actor],
                 opts[:authorize?],
                 query.tenant,
                 opts[:tracer],
                 query.resource,
                 query.domain,
                 parent_stack: parent_stack_from_context(query.context)
               )
         }

My idea is adding some kind of validation in the with chain before: :ok <- validate_expr_path(query).
Not sure if right approach but I can't figure out where else I could plug in the error.

@zachdaniel
Copy link
Contributor

Yeah, that would make sense to me 😄

@ken-kost
Copy link
Contributor

Okay cool. One more technical question:
What's in expression is:
type(not_existing_resource_or_relationship.attribute, Ash.Type.String, [])
And I'm not sure how to break that down. 🤔
I was thinking of using Info module for checks but i need to get from the above resource NotExistingResourceOrRelationship and attribute :attribute right? What would be the clean way of getting that (if there is a clean way 😬)?

@zachdaniel
Copy link
Contributor

Oh, I would not try to an analyze the expression yourself. just make sure that the error from hydrate_refs gets returned instead of a pattern match error.

@ken-kost
Copy link
Contributor

ken-kost commented Feb 25, 2025

Yea, that does make more sense. ⭐
Which would mean that instead of

         query <- %{
           query
           | filter:
               add_calc_context_to_filter(
                 query.filter,
                 opts[:actor],
                 opts[:authorize?],
                 query.tenant,
                 opts[:tracer],
                 query.domain,
                 query.resource,
                 parent_stack: parent_stack_from_context(query.context)
               ),
             sort:
               add_calc_context_to_sort(
                 query,
                 opts[:actor],
                 opts[:authorize?],
                 query.tenant,
                 opts[:tracer],
                 query.resource,
                 query.domain,
                 parent_stack: parent_stack_from_context(query.context)
               )

I will put add_calc_context_to_filter and add_calc_context_to_sort into the with chain above and they should return {:ok filter} and {:ok, sort} and will also have their with chains.
Yea, that's juicy. 🐛 Would that be the right approach? I'll assume it is.

@zachdaniel
Copy link
Contributor

Yeah, that code is pretty convoluted as well, so make sure that any other usages of those functions are updated. Are they public? I forget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Someday
Development

No branches or pull requests

3 participants