absinthe_plug icon indicating copy to clipboard operation
absinthe_plug copied to clipboard

Handle "unavailable" schema module status more correctly

Open giddie opened this issue 2 years ago • 6 comments

I've been dancing around these unpredictable "is not a valid Absinthe.Schema" errors for months now, and finally took some time to figure out exactly what's going on. It turns out that when checking the validity of the schema with Code.ensure_compiled/1, it's possible that the module is not available yet, but may be later. We don't currently handle that condition, so if the compiler hits a particular race condition, the schema is simply rejected. This little tweak seems to fix my issues completely :)

The Elixir documentation specifies for Code.ensure_compiled/1 that {:error, :unavailable} is not necessarily an error condition, as the deadlock could still be broken by the compiler. So we give the compiler a little time to break the deadlock and retry a number of times before we give up.

giddie avatar May 02 '22 16:05 giddie

Anything you'd like me to do to help get this merged?

giddie avatar Jul 04 '22 14:07 giddie

@benwilson512 This fixes a pretty severe bug. It particularly affects users that need to pull in functions from other modules (e.g. values for enums). The race condition can completely break schema compilation, and the resulting error is very obscure.

Given that the fix is so simple, can we get this merged?

giddie avatar Aug 10 '22 10:08 giddie

What version of Elixir are you on?

benwilson512 avatar Aug 10 '22 11:08 benwilson512

Elixir 1.13.0

giddie avatar Aug 10 '22 14:08 giddie

So, FYI, from the Elixir docs:

If it succeeds in loading the module, it returns {:module, module}. If not, returns {:error, reason} with the error reason. If the module being checked is currently in a compiler deadlock, this function returns {:error, :unavailable}. Unavailable doesn't necessarily mean the module doesn't exist, just that it is not currently available, but it (or may not) become available in the future.

https://hexdocs.pm/elixir/Code.html#ensure_compiled/1

The :unavailable situation is the one that is currently not handled, and this PR introduces the check required to handle it correctly.

For what it's worth, I've been using this patch in production for several months now, and it's completely resolved the schema build issues we were having, and doesn't seem to have introduced any new issues.

giddie avatar Sep 28 '22 09:09 giddie

@kdawgwilk Any chance we could get this merged?

giddie avatar Apr 12 '23 10:04 giddie