fix: convert DBConnection.EncodeError to an invalid-class error - #863
Conversation
`Ash.Type.Integer` accepts any Elixir integer, which is right for a type shared by data layers without a 64-bit limit, so an integer past the `bigint` range reaches Postgrex, which raises `DBConnection.EncodeError`. `handle_raised_error/4` had no clause for it and fell through to the generic one, so a read or a write with such a value returned `Ash.Error.Unknown` and the API extensions answered 500. The error carries only text, so the value is read back from the message. In a query it becomes `InvalidFilterValue`; in a changeset it becomes `InvalidAttribute` naming the attribute whose change holds the value, or `InvalidChanges` when the changeset is not available, as in the bulk create path. Closes ash-project#853
|
Now that #864 is in, this conflicted on the one hunk the description predicted, so I merged The three red checks are the same ones as on |
Reading the value and the expected range back out of Postgrex's message text depends on wording that is not part of any interface. The clause now maps by context alone, with a fixed message: `InvalidFilterValue` for a query and `InvalidChanges` for a changeset. The value and the attribute are no longer reported.
|
Makes sense. I've pushed a commit that drops the parsing: the clause now maps by context alone with a fixed message, Two things worth knowing with the parsing gone. |
|
I think we could potentially add a sentinel value to the error type, like |
|
Sounds good. Before replying I built the whole thing and ran it end to end, so this is the full change set as working code. Each piece is one commit on a branch in my forks, linked below, ready to open as PRs in order if you're happy with the shape. 1.
With the default nothing changes, so 2. 3. The renderers that build their own text from
Each reads the field defensively, so the code behaves as before on an 4. End to end, a filter on
The order would be: the What this doesn't change: the value and attribute from #853 are still not reported, and every Two things I ran into along the way, separate from this change:
|
|
@grempe can you include a change here to |
|
🚀 Thank you for your contribution! 🚀 |
Closes #853.
Ash.Type.Integeraccepts any Elixir integer, which is right for a type shared by data layers without a 64-bit limit, so an integer past thebigintrange reaches Postgrex, which raisesDBConnection.EncodeErrorwhen it encodes the parameter.handle_raised_error/4has no clause for that error and falls through to the generic one, soAsh.Query.filter(Post, score == ^9_223_372_036_854_775_808) |> Ash.read(), the same insidein, and a create or update with that value all returnAsh.Error.Unknown, and AshJsonApi / AshGraphql answer 500 / "Something went wrong".This adds a clause next to the existing
Ecto.Query.CastErrorone. The error carries only a message, so the value and the expected range are read back from Postgrex's text (Postgrex expected an integer in -9223372036854775808..9223372036854775807, got 9223372036854775808. Please make sure ...); if the text does not match that shape the generic handling is kept.Ash.Error.Query.InvalidFilterValuewithvalue: 9223372036854775808andmessage: "expected an integer in -9223372036854775808..9223372036854775807". Unlike theCastErrorclause it does not put the Ecto query incontext, since that ends up interpolated into the error's message.{:ecto_changeset, _, changeset}context it becomesAsh.Error.Changes.InvalidAttributenaming the attribute whose change holds the value, so an update reportsfield: :score.{:bulk_create, _}context the changeset built for the rescue has no changes, so it becomesAsh.Error.Changes.InvalidChangeswith the value and message.Tests:
test/encode_error_test.exscovers a filter, an update (asserting the field), a create, and an in-range control at the upper bound. Onmainthe three error cases returnAsh.Error.Unknownwrapping theEncodeError; with the change they pass. Full suite 1005 passed onmainand 982 passed with the patch on v2.13.1; the three failures onmainare theJoinSubquerySortTest/UniqAggregateSortTestcases from #858 that fail onmainwithout this change.mix format --check-formatted,mix credo --strict,mix sobelowandmix dialyzerclean.One thing I left out: with this repo's test repo,
score in ^[9_223_372_036_854_775_808]does not reach the encoder at all; the customanyfunction thatinis compiled to rejects the arguments on the Postgres side (function custom_any(bigint, numeric[]) does not exist). Against a plain repo theincase raises the sameEncodeErrorand is fixed by this change (it is one of the cases in the reproduction linked below), so I did not add a test for it here.Found by an AI agent working with a human fuzz-testing their own application; the reproduction outside this repo is https://github.com/grempe/ash-fuzz-repros/blob/main/test/ash_postgres/integer_past_64_bits_test.exs.
Contributor checklist
Leave anything that you believe does not apply unchecked.