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

Handle "unavailable" schema module status more correctly #266

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

giddie
Copy link

@giddie giddie commented May 2, 2022

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.

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
Copy link
Author

giddie commented Jul 4, 2022

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

@giddie
Copy link
Author

giddie commented Aug 10, 2022

@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?

@benwilson512
Copy link
Contributor

What version of Elixir are you on?

@giddie
Copy link
Author

giddie commented Aug 10, 2022

Elixir 1.13.0

@giddie
Copy link
Author

giddie commented Sep 28, 2022

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
Copy link
Author

giddie commented Apr 12, 2023

@kdawgwilk Any chance we could get this merged?

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.

None yet

3 participants