DmitryTsepelev / graphql-ruby-persisted_queries

Persisted queries for graphql-ruby
MIT License
171 stars 20 forks source link

Compiled Queries are not saved in `1.6.0` #58

Closed tgturner-shopify closed 1 year ago

tgturner-shopify commented 2 years ago

graphql (1.13.16) graphql-ruby-persisted_queries (1.6.0)

Today I was exploring graphql-ruby-persisted_queries but could not for the life of me get my queries to ever save to Redis. After doing some digging around, I noticed that before_query was being called after prepare_ast.

        def prepare_ast
          return super unless @context[:extensions]

          super.tap do
            if @context.errors.any?(&method(:not_found_error?))
              @context.errors.select!(&method(:not_found_error?))
            end

            resolver.persist(query_string, @document) if @not_loaded_document && query_string
          end
        end

Because of that, in the following line:

resolver.persist(query_string, @document) if @not_loaded_document && query_string

@not_loaded_document always returned nil since the expectation is that the before_query callback sets it, but that callback has not been called yet, and my queries were never saved.

After downgrading the gem to 1.5.1 everything appears to be working like a charm.

It is definitely possible that there is some setup error on my end.

DmitryTsepelev commented 2 years ago

Hi @tgturner-shopify!

I added 1.13.6 to the matrix (#59) and specs are passing. I tried to log from #before_query and #prepare_ast and the order is right. Could you please try to do the same? (in order to log prepare_ast you need to do EDITOR=code bundle open graphql, open up lib/graphql/query.rb and add logging)

tgturner-shopify commented 2 years ago

There definitely might be something strange going on in our setup, will look into this on our end! I'll close this out and re-open if we find anything.

DmitryTsepelev commented 1 year ago

@tgturner-shopify I found it! It might happen when someone accesses query manually, for instance when there is a tracer that catches "execute_multiplex" event. I'll try to make it work or just yank the last version 😌

tgturner-shopify commented 1 year ago

Thanks for keeping us in the loop!