Attempting to fix mysql issue by changing our usage of mysql2 to use the promise version, making sure disconnection always occurs correctly and using a slightly different syntax/approach.

5 years ago · 07551c54c1
2 changed files with 93 additions and 86 deletions
--- a/packages/server/src/integrations/mysql.ts
+++ b/packages/server/src/integrations/mysql.ts
@ -16,7 +16,7 @@ import {
 import { DatasourcePlus } from "./base/datasourcePlus"

 module MySQLModule {
-  const mysql = require("mysql2")
+  const mysql = require("mysql2/promise")
  const Sql = require("./base/sql")

  interface MySQLConfig {
@ -29,7 +29,7 @@ module MySQLModule {
  }

  const SCHEMA: Integration = {
-    docs: "https://github.com/mysqljs/mysql",
+    docs: "https://github.com/sidorares/node-mysql2",
    plus: true,
    friendlyName: "MySQL",
    description:
@ -80,36 +80,9 @@ module MySQLModule {
    },
  }

-  function internalQuery(
-    client: any,
-    query: SqlQuery,
-    connect: boolean = true
-  ): Promise<any[] | any> {
-    // Node MySQL is callback based, so we must wrap our call in a promise
-    return new Promise((resolve, reject) => {
-      if (connect) {
-        client.connect()
-      }
-      return client.query(
-        query.sql,
-        query.bindings || {},
-        (error: any, results: object[]) => {
-          if (error) {
-            reject(error)
-          } else {
-            resolve(results)
-          }
-          if (connect) {
-            client.end()
-          }
-        }
-      )
-    })
-  }
-
  class MySQLIntegration extends Sql implements DatasourcePlus {
    private config: MySQLConfig
-    private readonly client: any
+    private client: any
    public tables: Record<string, Table> = {}
    public schemaErrors: Record<string, string> = {}

@ -119,93 +92,127 @@ module MySQLModule {
      if (config.ssl && Object.keys(config.ssl).length === 0) {
        delete config.ssl
      }
-      this.client = mysql.createConnection(config)
+      this.config = config
+    }
+
+    async connect() {
+      this.client = await mysql.createConnection(this.config)
+    }
+
+    async disconnect() {
+      await this.client.end()
+    }
+
+    async internalQuery(
+      query: SqlQuery,
+      connect: boolean = true
+    ): Promise<any[] | any> {
+      try {
+        if (connect) {
+          await this.connect()
+        }
+        // Node MySQL is callback based, so we must wrap our call in a promise
+        const response = await this.client.query(
+          query.sql,
+          query.bindings || []
+        )
+        return response[0]
+      } finally {
+        if (connect) {
+          await this.disconnect()
+        }
+      }
    }

    async buildSchema(datasourceId: string, entities: Record<string, Table>) {
      const tables: { [key: string]: Table } = {}
      const database = this.config.database
-      this.client.connect()
-
-      // get the tables first
-      const tablesResp = await internalQuery(
-        this.client,
-        { sql: "SHOW TABLES;" },
-        false
-      )
-      const tableNames = tablesResp.map(
-        (obj: any) =>
-          obj[`Tables_in_${database}`] ||
-          obj[`Tables_in_${database.toLowerCase()}`]
-      )
-      for (let tableName of tableNames) {
-        const primaryKeys = []
-        const schema: TableSchema = {}
-        const descResp = await internalQuery(
-          this.client,
-          { sql: `DESCRIBE \`${tableName}\`;` },
+      await this.connect()
+
+      try {
+        // get the tables first
+        const tablesResp = await this.internalQuery(
+          { sql: "SHOW TABLES;" },
          false
        )
-        for (let column of descResp) {
-          const columnName = column.Field
-          if (column.Key === "PRI" && primaryKeys.indexOf(column.Key) === -1) {
-            primaryKeys.push(columnName)
-          }
-          const constraints = {
-            presence: column.Null !== "YES",
-          }
-          const isAuto: boolean =
-            typeof column.Extra === "string" &&
-            (column.Extra === "auto_increment" ||
-              column.Extra.toLowerCase().includes("generated"))
-          schema[columnName] = {
-            name: columnName,
-            autocolumn: isAuto,
-            type: convertSqlType(column.Type),
-            constraints,
+        const tableNames = tablesResp.map(
+          (obj: any) =>
+            obj[`Tables_in_${database}`] ||
+            obj[`Tables_in_${database.toLowerCase()}`]
+        )
+        for (let tableName of tableNames) {
+          const primaryKeys = []
+          const schema: TableSchema = {}
+          const descResp = await this.internalQuery(
+            { sql: `DESCRIBE \`${tableName}\`;` },
+            false
+          )
+          for (let column of descResp) {
+            const columnName = column.Field
+            if (
+              column.Key === "PRI" &&
+              primaryKeys.indexOf(column.Key) === -1
+            ) {
+              primaryKeys.push(columnName)
+            }
+            const constraints = {
+              presence: column.Null !== "YES",
+            }
+            const isAuto: boolean =
+              typeof column.Extra === "string" &&
+              (column.Extra === "auto_increment" ||
+                column.Extra.toLowerCase().includes("generated"))
+            schema[columnName] = {
+              name: columnName,
+              autocolumn: isAuto,
+              type: convertSqlType(column.Type),
+              constraints,
+            }
          }
-        }
-        if (!tables[tableName]) {
-          tables[tableName] = {
-            _id: buildExternalTableId(datasourceId, tableName),
-            primary: primaryKeys,
-            name: tableName,
-            schema,
+          if (!tables[tableName]) {
+            tables[tableName] = {
+              _id: buildExternalTableId(datasourceId, tableName),
+              primary: primaryKeys,
+              name: tableName,
+              schema,
+            }
          }
        }
+      } finally {
+        await this.disconnect()
      }
-
-      this.client.end()
      const final = finaliseExternalTables(tables, entities)
      this.tables = final.tables
      this.schemaErrors = final.errors
    }

    async create(query: SqlQuery | string) {
-      const results = await internalQuery(this.client, getSqlQuery(query))
+      const results = await this.internalQuery(getSqlQuery(query))
      return results.length ? results : [{ created: true }]
    }

    async read(query: SqlQuery | string) {
-      return internalQuery(this.client, getSqlQuery(query))
+      return this.internalQuery(getSqlQuery(query))
    }

    async update(query: SqlQuery | string) {
-      const results = await internalQuery(this.client, getSqlQuery(query))
+      const results = await this.internalQuery(getSqlQuery(query))
      return results.length ? results : [{ updated: true }]
    }

    async delete(query: SqlQuery | string) {
-      const results = await internalQuery(this.client, getSqlQuery(query))
+      const results = await this.internalQuery(getSqlQuery(query))
      return results.length ? results : [{ deleted: true }]
    }

    async query(json: QueryJson) {
-      this.client.connect()
-      const queryFn = (query: any) => internalQuery(this.client, query, false)
-      const output = await this.queryWithReturning(json, queryFn)
-      this.client.end()
-      return output
+      await this.connect()
+      try {
+        const queryFn = (query: any) => this.internalQuery(query, false)
+        return await this.queryWithReturning(json, queryFn)
+      } finally {
+        await this.disconnect()
+      }
    }
  }

--- a/packages/server/src/integrations/utils.ts
+++ b/packages/server/src/integrations/utils.ts
@ -40,7 +40,7 @@ const SQL_TYPE_MAP = {
 export enum SqlClients {
  MS_SQL = "mssql",
  POSTGRES = "pg",
-  MY_SQL = "mysql",
+  MY_SQL = "mysql2",
  ORACLE = "oracledb",
 }