Skip to content

fix: convert DBConnection.EncodeError to an invalid-class error - #863

Merged
zachdaniel merged 3 commits into
ash-project:mainfrom
grempe:fix/encode-error-conversion
Sep 24, 2026
Merged

zachdaniel merged 3 commits into
ash-project:mainfrom
grempe:fix/encode-error-conversion

Conversation

@grempe

@grempe grempe commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Closes #853.

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 when it encodes the parameter. handle_raised_error/4 has no clause for that error and falls through to the generic one, so Ash.Query.filter(Post, score == ^9_223_372_036_854_775_808) |> Ash.read(), the same inside in, and a create or update with that value all return Ash.Error.Unknown, and AshJsonApi / AshGraphql answer 500 / "Something went wrong".

This adds a clause next to the existing Ecto.Query.CastError one. 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.

  • In a query context it becomes Ash.Error.Query.InvalidFilterValue with value: 9223372036854775808 and message: "expected an integer in -9223372036854775808..9223372036854775807". Unlike the CastError clause it does not put the Ecto query in context, since that ends up interpolated into the error's message.
  • In an {:ecto_changeset, _, changeset} context it becomes Ash.Error.Changes.InvalidAttribute naming the attribute whose change holds the value, so an update reports field: :score.
  • In the {:bulk_create, _} context the changeset built for the rescue has no changes, so it becomes Ash.Error.Changes.InvalidChanges with the value and message.

Tests: test/encode_error_test.exs covers a filter, an update (asserting the field), a create, and an in-range control at the upper bound. On main the three error cases return Ash.Error.Unknown wrapping the EncodeError; with the change they pass. Full suite 1005 passed on main and 982 passed with the patch on v2.13.1; the three failures on main are the JoinSubquerySortTest / UniqAggregateSortTest cases from #858 that fail on main without this change. mix format --check-formatted, mix credo --strict, mix sobelow and mix dialyzer clean.

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 custom any function that in is compiled to rejects the arguments on the Postgres side (function custom_any(bigint, numeric[]) does not exist). Against a plain repo the in case raises the same EncodeError and 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.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

`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
@grempe

grempe commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Now that #864 is in, this conflicted on the one hunk the description predicted, so I merged main into the branch and kept both handle_raised_error/4 clauses (the second commit here). It is mergeable again, and both test/encode_error_test.exs and test/cast_error_test.exs pass on the merged result along with the rest of the suite.

The three red checks are the same ones as on main's own run for #864: mix test fails only the three JoinSubquerySortTest / UniqAggregateSortTest cases from #858, and audit and REUSE compliance fail identically there. Everything else, including dialyzer, is green on all five PostgreSQL versions.

Comment thread lib/data_layer.ex Outdated
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.
@grempe

grempe commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Makes sense. I've pushed a commit that drops the parsing: 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, so an update no longer names score, and the tests assert only the error class. Suite, credo, dialyzer and the reproduction are unchanged otherwise.

Two things worth knowing with the parsing gone. InvalidFilterValue with no value renders as Invalid filter value `nil`: ..., since its message/1 always inspects the value. And every EncodeError now becomes invalid-class, including ones that are really configuration errors, such as an anonymous tuple with no Postgrex extension, whose original message is lost. Happy to adjust either if you'd rather.

@zachdaniel

Copy link
Copy Markdown
Contributor

I think we could potentially add a sentinel value to the error type, like no_value?: true to tell it to use a different message. We could make that change in ash, and get this in after that is released using that new option. WDYT?

@grempe

grempe commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

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. ash: the option (branch). Ash.Error.Query.InvalidFilterValue gets a no_value?: false field. When it is true, message/1 leaves the value out:

  • Invalid filter value: <message>, or Invalid filter value. with no message
  • Invalid filter value supplied in <context>: <message> when there is a context

With the default nothing changes, so nil is still rendered as nil when it really is the value. The moduledoc explains when to use it. There are four new tests, and the full suite, credo and dialyzer pass.

2. ash_postgres: #863 and #866 set it (one commit on top of each: #863, #866). Only the query case changes: the encode error in #863 and the 22021 error in #866 now build InvalidFilterValue with no_value?: true. Both PRs will need their ash requirement raised to the release that has the option. With both on main and pointed at the local ash, the full suite passes apart from the three #858 tests.

3. The renderers that build their own text from value. Without this, they still show nil:

Each reads the field defensively, so the code behaves as before on an ash without it and these can land in any order. The one exception is each PR's new test: it builds the error with no_value?: true, so it needs the new ash. All three full suites pass against the local ash.

4. ash_json_api needs nothing. The InvalidFilterValue impl on main (9f86eba) calls message/1, so it picks this up directly. 1.7.1 has no impl yet and answers a generic 400.

End to end, a filter on score with 2^63 and one on title with a NUL byte now show the text below. The #853 case can't be expressed in GraphQL, where Int rejects it first.

Surface Now With the change
Ash Invalid filter value `nil`: a value does not fit the type of its column Invalid filter value: a value does not fit the type of its column
GraphQL Invalid filter value nil: invalid byte sequence ... Invalid filter value: invalid byte sequence for encoding "UTF8": 0x00
JSON:API (main) Invalid filter value `nil`: ... Invalid filter value: ...
Lua vars.value = "nil" no value var; detail carries the message
MCP tool invalid filter value nil: ... invalid filter value: invalid byte sequence ...

The order would be: the ash PR, then the three renderer PRs whenever, then #863 and #866 once the ash release is out.

What this doesn't change: the value and attribute from #853 are still not reported, and every EncodeError, including configuration errors, still becomes invalid-class. That's the same as the current #863.

Two things I ran into along the way, separate from this change:

  • On ash_json_api main, the new InvalidFilterValue impl renders message/1, which includes context. ash_postgres's cast-error clause puts the whole Ecto query there, so GET /posts?filter[id]=not-a-uuid returns a 3,400-character detail with the query and file paths in it. The GraphQL, Lua and MCP renderers avoid this by not rendering context. It isn't released yet. I'm happy to fix it on either side, whichever you prefer.
  • ash_lua's two "host exceptions during a call" tests fail against current ash main, on ash_lua main too. Since #2947 and #2951, the malformed filter and sort no longer raise there. They pass against the 3.33.1 that ash_lua's lock pins, so they'll show up at the next ash bump.

@zachdaniel

Copy link
Copy Markdown
Contributor

@grempe can you include a change here to ash_json_api to not use the message from InvalidFilterValue?

@zachdaniel
zachdaniel merged commit 2209c09 into ash-project:main Sep 24, 2026
111 of 126 checks passed
@zachdaniel

Copy link
Copy Markdown
Contributor

🚀 Thank you for your contribution! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An integer outside the bigint range is an unconverted DBConnection.EncodeError

2 participants