Detect when a connection is closed before it's done opening

- close connections explicitly in test code

Author: gionatamettifogo

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "sqlitecloud-js",
-  "version": "0.0.22-beta",
+  "version": "0.0.23",
   "description": "SQLiteCloud drivers for Typescript/Javascript in edge, web and node clients",
   "main": "./lib/index.js",
   "types": "./lib/index.d.ts",

package.json

@@ -2,13 +2,14 @@
  * connection.ts - handles low level communication with sqlitecloud server
  */
 
-import tls from 'tls'
+import tls, { TLSSocket } from 'tls'
 
 const lz4 = require('lz4js')
 
 import { SQLiteCloudConfig, SQLCloudRowsetMetadata, SQLiteCloudError, SQLiteCloudDataTypes, ErrorCallback, ResultsCallback } from './types'
 import { SQLiteCloudRowset } from './rowset'
 import { parseConnectionString, parseBoolean } from './utilities'
+import { Socket } from 'net'
 
 /**
  * The server communicates with clients via commands defined
@@ -59,7 +60,7 @@ export class SQLiteCloudConnection {
   private config: SQLiteCloudConfig
 
   /** Currently opened tls socket used to communicated with SQLiteCloud server */
-  private socket?: tls.TLSSocket
+  private socket?: tls.TLSSocket | null
 
   /** Operations are serialized by waiting an any pending promises */
   private operations = new OperationsQueue()
@@ -70,7 +71,7 @@ export class SQLiteCloudConnection {
 
   /** True if connection is open */
   public get connected(): boolean {
-    return this.socket !== undefined
+    return !!this.socket
   }
 
   //
@@ -154,7 +155,7 @@ export class SQLiteCloudConnection {
   private connect(callback?: ErrorCallback): this {
     this.operations.enqueue(done => {
       // connection established while we were waiting in line?
-      console.assert(this.socket === undefined, 'Connection already established')
+      console.assert(!this.connected, 'Connection already established')
 
       // clear all listeners and call done in the operations queue
       const finish: ResultsCallback = error => {
@@ -170,16 +171,23 @@ export class SQLiteCloudConnection {
       }
 
       // connect to tls socket, initialize connection, setup event handlers
-      const client: tls.TLSSocket = tls.connect(this.config.port as number, this.config.host, this.config.tlsOptions, () => {
-        if (!client.authorized) {
-          const anonimizedError = anonimizeError(client.authorizationError)
+      this.socket = tls.connect(this.config.port as number, this.config.host, this.config.tlsOptions, () => {
+        if (!this.socket?.authorized) {
+          const anonimizedError = anonimizeError((this.socket as TLSSocket).authorizationError)
           this.log('Connection was not authorized', anonimizedError)
           this.close()
           finish(new SQLiteCloudError('Connection was not authorized', { cause: anonimizedError }))
         } else {
-          this.socket = client
+          // the connection was closed before it was even opened,
+          // eg. client closed the connection before the server accepted it
+          if (this.socket === null) {
+            this.log('Connection was closed before it was opened')
+            finish(new SQLiteCloudError('Connection was closed before it was done opening'))
+            return
+          }
 
           // send initialization commands
+          console.assert(this.socket, 'Connection already closed')
           const commands = this.initializationCommands
           this.processCommands(commands, error => {
             if (error) {
@@ -194,17 +202,18 @@ export class SQLiteCloudConnection {
         }
       })
 
-      client.on('close', () => {
+      this.socket.on('close', () => {
         this.log('Connection closed')
-        this.socket = undefined
+        this.socket = null
         finish(new SQLiteCloudError('Connection was closed'))
       })
 
-      client.once('error', (error: any) => {
+      this.socket.once('error', (error: any) => {
         this.log('Connection error', error)
         finish(new SQLiteCloudError('Connection error', { cause: error }))
       })
     })
+
     return this
   }
 
@@ -348,8 +357,10 @@ export class SQLiteCloudConnection {
 
   /** Disconnect from server, release connection. */
   public close(): this {
+    console.assert(this.socket !== undefined, 'SQLiteCloudConnection.close - connection already closed')
     if (this.socket) {
       this.socket.destroy()
+      this.socket = null
     }
     this.operations.clear()
     this.socket = undefined

src/connection.ts

@@ -8,6 +8,7 @@ import { getChinookDatabase, getTestingDatabase } from './shared'
 // https://github.com/TryGhost/node-sqlite3/wiki/API
 import sqlite3 from 'sqlite3'
 import { join } from 'path'
+import { error } from 'console'
 
 const INSERT_SQL = "INSERT INTO people (name, hobby, age) VALUES ('Fantozzi Ugo', 'Competitive unicorn farting', 42); "
 const TESTING_DATABASE_FILE = join(__dirname, 'assets/testing.db')
@@ -81,14 +82,30 @@ describe('Database.on', () => {
     chinookFile.close()
   })
 
-  it('sqlitecloud: should emit close event', done => {
+  it('sqlitecloud: should close before it finishes opening', done => {
     const chinookCloud = getChinookDatabase()
     chinookCloud.once('close', () => {
       done()
     })
+
+    // we are closing the connection before it's really had a chance to open...
     chinookCloud.close()
   })
 
+  it('sqlitecloud: should emit close event', done => {
+    const chinookCloud = getChinookDatabase((error, database) => {
+      // we first wait for the database to open, then close it
+      expect(error).toBeNull()
+
+      chinookCloud.once('close', () => {
+        done()
+      })
+
+      // we are closing the connection asynchonously
+      chinookCloud.close()
+    })
+  })
+
   // end Database.on
 })
 

test/compare.test.ts

@@ -4,25 +4,20 @@
 
 import { SQLiteCloudError } from '../src/index'
 import { SQLiteCloudConnection, anonimizeCommand } from '../src/connection'
-import { parseConnectionString } from '../src/utilities'
 import {
   CHINOOK_DATABASE_URL,
   LONG_TIMEOUT,
   getTestingConfig,
   getChinookConfig,
   getChinookConnection,
+  // clearTestingDatabasesAsync,
   WARN_SPEED_MS,
   EXPECT_SPEED_MS
-  // clearTestingDatabasesAsync
 } from './shared'
 
 describe('connection', () => {
   let chinook: SQLiteCloudConnection
-  /*
-  beforeAll(async () => {
-    
-  })
-*/
+
   beforeEach(() => {
     chinook = getChinookConnection()
   })
@@ -37,13 +32,15 @@ describe('connection', () => {
     it('should connect', () => {
       // ...in beforeEach
     })
-
     /*
-    it('should run manually once in a while to clear orphaned databases', async () => {
-      await clearTestingDatabasesAsync()
-    })
-    */
-
+    it(
+      'should drop all old testing databases',
+      async () => {
+        await clearTestingDatabasesAsync()
+      },
+      LONG_TIMEOUT
+    )
+*/
     it('should add self signed certificate for localhost connections', () => {
       const localConfig = getChinookConfig('sqlitecloud://admin:xxx@localhost:8850/chinook.db')
       expect(localConfig.host).toBe('localhost')

test/connection.test.ts

@@ -77,6 +77,7 @@ describe('stress testing', () => {
       const connections: Database[] = []
 
       try {
+        // open connection
         for (let i = 0; i < SIMULTANEOUS_TEST_SIZE; i++) {
           connections.push(
             getChinookDatabase(error => {
@@ -87,12 +88,16 @@ describe('stress testing', () => {
             })
           )
         }
+
+        // use connection
         for (let i = 0; i < SIMULTANEOUS_TEST_SIZE; i++) {
           const connection = connections[i]
           expect(connection).not.toBeNull()
           const results = await connection.sql`SELECT ${i} as 'connection_id'`
           expect(results[0]['connection_id']).toBe(i)
         }
+
+        // close connection
         for (let i = 0; i < SIMULTANEOUS_TEST_SIZE; i++) {
           connections[i].close()
         }
@@ -115,10 +120,11 @@ describe('stress testing', () => {
     async () => {
       const numQueries = 20
       const startTime = Date.now()
-      const database = getChinookDatabase()
+      const chinook = getChinookDatabase()
+
       const table = 'tracks'
       for (let i = 0; i < SEQUENCE_TEST_SIZE; i++) {
-        const results = await database.sql`SELECT * FROM ${table} ORDER BY RANDOM() LIMIT 12`
+        const results = await chinook.sql`SELECT * FROM ${table} ORDER BY RANDOM() LIMIT 12`
         expect(results).toHaveLength(12)
         expect(Object.keys(results[0])).toEqual(['TrackId', 'Name', 'AlbumId', 'MediaTypeId', 'GenreId', 'Composer', 'Milliseconds', 'Bytes', 'UnitPrice'])
       }
@@ -128,6 +134,8 @@ describe('stress testing', () => {
         console.warn(`${numQueries}x database.sql selects, ${queryMs.toFixed(0)}ms per query`)
         expect(queryMs).toBeLessThan(EXPECT_SPEED_MS)
       }
+
+      chinook.close()
     },
     EXTRA_LONG_TIMEOUT
   )
@@ -140,6 +148,8 @@ describe('stress testing', () => {
     expect(smallResults).toHaveLength(1)
     expect(smallResults.metadata.columns[0].name).toBe('small')
     expect(smallResults[0].small).toHaveLength(2 * 1000) // hex encoded
+
+    chinook.close()
   })
 
   it(
@@ -153,6 +163,8 @@ describe('stress testing', () => {
       expect(largeResults.metadata.columns[0].name).toBe('columName')
       const largeResultString = largeResults[0].columName as string
       expect(largeResultString).toHaveLength(2 * 10000000) // 20 MB
+
+      chinook.close()
     },
     EXTRA_LONG_TIMEOUT
   )
@@ -171,14 +183,16 @@ describe('stress testing', () => {
       expect(largeCompressedResults).toHaveLength(1)
       expect(largeCompressedResults.metadata.columns[0].name).toBe('columnName')
       expect(largeCompressedResults[0].columnName).toHaveLength(2 * blobSize)
+
+      chinook.close()
     },
     EXTRA_LONG_TIMEOUT
   )
 
   it(
     'should receive large responses with compressed data',
     async () => {
-      const chinook = getChinookDatabase()
+      const chinook = getChinookDatabase(undefined, { timeout: EXTRA_LONG_TIMEOUT })
 
       // enable compression
       const enable = await chinook.sql`SET CLIENT KEY COMPRESSION TO 1;`
@@ -193,6 +207,8 @@ describe('stress testing', () => {
       const postfix = await chinook.sql`SELECT 1`
       expect(postfix).toHaveLength(1)
       expect(postfix[0]['1']).toBe(1)
+
+      chinook.close()
     },
     EXTRA_LONG_TIMEOUT
   )