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

QueryManager error: listener is not a function #231

Closed
bruce opened this issue May 21, 2016 · 4 comments
Closed

QueryManager error: listener is not a function #231

bruce opened this issue May 21, 2016 · 4 comments

Comments

@bruce
Copy link
Contributor

bruce commented May 21, 2016

I've talked/worked through part of this -- after posting a gist -- with @stubailo

Encountering the following error during query execution:

(program):12 TypeError: listener is not a function
    at eval (eval at <anonymous> (main.js:3742), <anonymous>:300:13)
    at eval (eval at <anonymous> (main.js:5308), <anonymous>:40:11)
    at baseForOwn (eval at <anonymous> (main.js:5332), <anonymous>:22:20)
    at forOwn (eval at <anonymous> (main.js:5332), <anonymous>:54:20)
    at QueryManager.broadcastQueries (eval at <anonymous> (main.js:3742), <anonymous>:297:7)
    at Array.eval (eval at <anonymous> (main.js:3742), <anonymous>:53:23)
    at dispatch (eval at <anonymous> (main.js:3640), <anonymous>:180:19)
    at Object.eval [as dispatch] (eval at <anonymous> (main.js:4108), <anonymous>:8:16)
    at Object.dispatch (<anonymous>:1289:344)
    at eval (eval at <anonymous> (main.js:3742), <anonymous>:231:29)

After inserting some console.log entries QueryManager.js:

    QueryManager.prototype.broadcastQueries = function () {
      var queries = this.getApolloState().queries;
      console.log({listeners: this.queryListeners, queries: queries});
      forOwn(this.queryListeners, function (listener, queryId) {
            console.log('queryId', queryId);
            var queryStoreValue = queries[queryId];
            listener(queryStoreValue);
        });
    };

I notice that listeners and queries both have keys for query IDs 0 and 6 -- but, where the queryId is logged, an additional query ID, 4 is output.

Checking further up the console log (and checking redux dev tools), I do see a APOLLO_QUERY_STOP for query ID 4.

@bruce
Copy link
Contributor Author

bruce commented May 21, 2016

Confirmed that guarding the listener invocation with if (listener) { ... } fixes my application issue, so this appears to be (from a layman's view) some housecleaning debris (race condition?) that just needs to be cleaned up.

@bruce
Copy link
Contributor Author

bruce commented May 22, 2016

I've verified interleaving of stopQuery() and broadcastQueries() that's seemingly leading to a race condition. Logging at the beginning and ending of each function (and which query IDs are present on this.queryListeners), I'm seeing:

BEGIN: broadcastQueries()
- this.queryListeners has queryId = 0 with a listener 
BEGIN: stopQuery() with queryId = 2
BEGIN: broadcastQueries()
- this.queryListeners has queryId = 0 with a listener 
END: broadcastQueries()
END: stopQuery() with queryId = 2
BEGIN: startQuery()
BEGIN: broadcastQueries()
- this.queryListeners has queryId = 0 with a listener 
- this.queryListeners has queryId = 4 with a listener 
END: broadcastQueries()
BEGIN: broadcastQueries()
- this.queryListeners has queryId = 0 with a listener 
- this.queryListeners has queryId = 4 with a listener 
END: broadcastQueries()
END: startQuery()
- this.queryListeners has queryId = 2 WITHOUT a listener (listener is undefined).
(ERROR)

Interestingly, it seems the use of forOwn is the problem. This code works perfectly fine:

for (var queryId in this.queryListeners) {
  if (this.queryListeners.hasOwnProperty(queryId)) {
    var queryStoreValue = queries[queryId];
    var listener = this.queryListeners[queryId];
    listener(queryStoreValue);
  }
}

It appears the forOwn use of a callback function, being asynchronously executed, causes the issue.

I would happily provide a PR that either:

  • Explicitly checks listener and retains the use of forOwn
  • Removes the use of forOwn and uses for (var in ...) { ... }

I would guess, based on the use of forOwn elsewhere, that the first option is likely the best choice.

@stubailo
Copy link
Contributor

Thank you for identifying the issue and adding the PR!

@stubailo
Copy link
Contributor

Published the fix in 0.3.10!

jbaxleyiii pushed a commit that referenced this issue Oct 18, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants