fix: bindings now error when closed during empty writes (#1872)

Also fix tests not running!

Author: reconbot

packages/binding-abstract/binding-abstract.js

@@ -119,6 +119,8 @@ The in progress writes must error when the port is closed with an error object t
 
     debug('write', buffer.length, 'bytes')
     if (!this.isOpen) {
+      debug('write', 'error port is not open')
+
       throw new Error('Port is not open')
     }
   }

packages/bindings/lib/bindings-test.js renamed to packages/bindings/lib/bindings.test.js

@@ -38,6 +38,17 @@ function disconnect(err) {
   throw err || new Error('Unknown disconnection')
 }
 
+function shouldReject(promise, errType = Error, message = 'Should have rejected') {
+  return promise.then(
+    () => {
+      throw new Error(message)
+    },
+    err => {
+      assert.instanceOf(err, errType)
+    }
+  )
+}
+
 // All bindings are required to work with an "echo" firmware
 // The echo firmware should respond with this data when it's
 // ready to echo. This allows for remote device bootup.
@@ -156,22 +167,12 @@ function testBinding(bindingName, Binding, testPort) {
           })
         })
 
-        it('throws when not given a path', done => {
-          try {
-            binding.open('')
-          } catch (e) {
-            assert.instanceOf(e, TypeError)
-            done()
-          }
+        it('throws when not given a path', async () => {
+          await shouldReject(binding.open(''), TypeError)
         })
 
-        it('throws when not given options', done => {
-          try {
-            binding.open('COMBAD')
-          } catch (e) {
-            assert.instanceOf(e, TypeError)
-            done()
-          }
+        it('throws when not given options', async () => {
+          await shouldReject(binding.open('COMBAD'), TypeError)
         })
 
         if (!testPort) {
@@ -278,15 +279,9 @@ function testBinding(bindingName, Binding, testPort) {
       })
 
       describe('#update', () => {
-        it('throws when not given an object', done => {
+        it('throws when not given an object', async () => {
           const binding = new Binding({ disconnect })
-
-          try {
-            binding.update()
-          } catch (e) {
-            assert.instanceOf(e, TypeError)
-            done()
-          }
+          await shouldReject(binding.update(), TypeError)
         })
 
         it('errors asynchronously when not open', done => {
@@ -315,53 +310,47 @@ function testBinding(bindingName, Binding, testPort) {
 
         afterEach(() => binding.close())
 
-        it('throws errors when updating nothing', done => {
-          try {
-            binding.update({})
-          } catch (err) {
-            assert.instanceOf(err, Error)
-            done()
-          }
+        it('throws errors when updating nothing', async () => {
+          await shouldReject(binding.update({}), Error)
         })
 
-        it('errors when not called with options', done => {
-          try {
-            binding.set(() => {})
-          } catch (e) {
-            assert.instanceOf(e, Error)
-            done()
-          }
+        it('errors when not called with options', async () => {
+          await shouldReject(binding.set(() => {}), Error)
         })
 
         it('updates baudRate', () => {
           return binding.update({ baudRate: 57600 })
         })
       })
 
-      describe('#write', () => {
+      describe.only('#write', () => {
         it('errors asynchronously when not open', done => {
           const binding = new Binding({
             disconnect,
           })
           let noZalgo = false
-          binding.write(Buffer.from([])).catch(err => {
-            assert.instanceOf(err, Error)
-            assert(noZalgo)
-            done()
-          })
+          binding
+            .write(Buffer.from([]))
+            .then(
+              data => {
+                console.log({ data })
+                throw new Error('Should have errored')
+              },
+              err => {
+                assert.instanceOf(err, Error)
+                assert(noZalgo)
+                done()
+              }
+            )
+            .catch(done)
           noZalgo = true
         })
 
-        it('throws when not given a buffer', done => {
+        it('throws when not given a buffer', async () => {
           const binding = new Binding({
             disconnect,
           })
-          try {
-            binding.write(null)
-          } catch (e) {
-            assert.instanceOf(e, TypeError)
-            done()
-          }
+          await shouldReject(binding.write(null), TypeError)
         })
 
         if (!testPort) {
@@ -494,16 +483,11 @@ function testBinding(bindingName, Binding, testPort) {
           noZalgo = true
         })
 
-        it('throws when not called with options', done => {
+        it('throws when not called with options', async () => {
           const binding = new Binding({
             disconnect,
           })
-          try {
-            binding.set(() => {})
-          } catch (e) {
-            assert.instanceOf(e, TypeError)
-            done()
-          }
+          await shouldReject(binding.set(() => {}), TypeError)
         })
 
         if (!testPort) {

packages/bindings/lib/darwin.js

@@ -65,12 +65,14 @@ class DarwinBinding extends AbstractBinding {
   }
 
   async write(buffer) {
-    if (buffer.length === 0) {
-      return
-    }
     this.writeOperation = super
       .write(buffer)
-      .then(() => unixWrite.call(this, buffer))
+      .then(() => {
+        if (buffer.length === 0) {
+          return
+        }
+        return unixWrite.call(this, buffer)
+      })
       .then(() => {
         this.writeOperation = null
       })

packages/bindings/lib/linux.js

@@ -65,12 +65,14 @@ class LinuxBinding extends AbstractBinding {
   }
 
   async write(buffer) {
-    if (buffer.length === 0) {
-      return
-    }
     this.writeOperation = super
       .write(buffer)
-      .then(() => unixWrite.call(this, buffer))
+      .then(() => {
+        if (buffer.length === 0) {
+          return
+        }
+        return unixWrite.call(this, buffer)
+      })
       .then(() => {
         this.writeOperation = null
       })

packages/bindings/lib/win32.js

@@ -74,12 +74,14 @@ class WindowsBinding extends AbstractBinding {
   }
 
   async write(buffer) {
-    if (buffer.length === 0) {
-      return
-    }
     this.writeOperation = super
       .write(buffer)
-      .then(() => asyncWrite(this.fd, buffer))
+      .then(() => {
+        if (buffer.length === 0) {
+          return
+        }
+        return asyncWrite(this.fd, buffer)
+      })
       .then(() => {
         this.writeOperation = null
       })