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

Fix for MySQL Pooling Async Bug #1710

Closed
Antzy opened this issue Apr 9, 2020 · 4 comments · Fixed by #1712
Closed

Fix for MySQL Pooling Async Bug #1710

Antzy opened this issue Apr 9, 2020 · 4 comments · Fixed by #1712
Assignees
Labels
api: cloudsql Issues related to the Cloud SQL for MySQL API. type: docs Improvement to the documentation for an API.

Comments

@Antzy
Copy link

Antzy commented Apr 9, 2020

The NodeJS MySQL server.js code has an async Pool creation call createPool() without any await. So when it is immediately followed by ensureSchema() which runs a query on the pool, it throws an error as the pool isn't yet created:

UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'query' of undefined

This is the same example on Google Cloud Docs and I assumed them to be correct and wasted 2 days on this thinking something wrong at my end. Please fix this so even any other call can ensure that pool object is properly created before call.

Link to code:
https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/master/cloud-sql/mysql/mysql/server.js

@Antzy
Copy link
Author

Antzy commented Apr 9, 2020

Might not be the optimum way but thus is how I solved it:

var createPoolPromise = createPool();
// [END cloud_sql_mysql_mysql_create]

const ensureSchema = async () => {
  await createPoolPromise;
  // Wait for tables to be created (if they don't already exist).
  await pool.query(
    `CREATE TABLE IF NOT EXISTS votes
      ( vote_id SERIAL NOT NULL, time_cast timestamp NOT NULL,
      candidate CHAR(6) NOT NULL, PRIMARY KEY (vote_id) );`
  );
};
ensureSchema();

I saved the returned Promise from createPool() and added await for it before every query. Not the prettiest solution but is there any better way?

@fhinkel
Copy link
Contributor

fhinkel commented Apr 9, 2020

@Antzy 👋 Thanks for raising the issue. We'll fix it!

@fhinkel fhinkel added type: docs Improvement to the documentation for an API. api: cloudsql Issues related to the Cloud SQL for MySQL API. labels Apr 9, 2020
@shubha-rajan
Copy link
Contributor

Thanks for raising the issue! I think using a middleware function to ensure pool creation might be a cleaner approach. The problem I see is that even if we await the promise to create the pool before each query, if table creation hasn't completed before those other queries, the other queries will fail. I think we should actually be awaiting the promise from ensureSchema before other queries. I opened #1712 with a possible fix

@Antzy
Copy link
Author

Antzy commented Apr 12, 2020

Thanks for looking into this. My schema is pretty much guaranteed to be already in place, so I was await-ing pool creation before every query. But you are right, much better to await ensureSchema instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudsql Issues related to the Cloud SQL for MySQL API. type: docs Improvement to the documentation for an API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants