Merge pull request #29 from keepkey/read-error-handling

Change `cancel` to bypass the message queue

example/index.html

@@ -29,8 +29,14 @@ <h3>WebUSB</h3>
       <p>To seed the <code>window.keepkeyManager</code> with WebUSB devices, first pair a device, then click start.</p>
       <button class="button button-outline" onclick="window.keepkey.WebUSBDevice.requestPair()">Pair WebUSB Device</button>
       <button onclick="connectWebUSB()">START WEBUSB</button>
+      <div>
+        <button onclick="ping()">Ping</button>
+        <button onclick="pingWithButton()">Ping With Button</button>
+        <button onclick="pingWithPIN()">Ping With PIN</button>
+        <button onclick="cancelPending()">Send Cancel</button>
+      </div>
     </div>
-  
+
   </div>
   <script src="https://unpkg.com/[email protected]/dist/debug.js"></script>
   <script src="https://code.jquery.com/jquery-3.3.1.min.js" integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8=" crossorigin="anonymous"></script>

example/js/main.js

@@ -126,6 +126,9 @@ window.connectWebUSB = function () {
         const k = manager.get()
         console.log('Putting first keepkey on window as window.keepkey: ', k)
         window.keepkey = k
+        k.device.events.on('write', data => console.log('Event: write', data))
+        k.device.events.on('read', data => console.log('Event: read', data))
+        k.device.events.on('reading', () => console.log('Event: reading'))
       }
       return manager.exec('getFeatures')
     })
@@ -147,3 +150,40 @@ window.connectWebUSB = function () {
       console.error(String(e))
     })
 }
+
+// So we can see which commands aren't resolving
+window.allPings = []
+
+const log = (name, p) => {
+  console.log('sending', name)
+  const details = { name, state: 'pending' }
+  details.p = p
+    .then(res => {
+      console.log(`${name} response:`, res)
+      details.state = 'resolved'
+    })
+    .catch(e => {
+      console.error(`${name} error:`, e)
+      details.state = 'rejected'
+    })
+  allPings.push(details)
+  return p
+}
+
+let pingCount = 0
+
+window.pingWithButton = function () {
+  log(`ping ${++pingCount} with button`, manager.exec('ping', { message: 'ping' + pingCount, buttonProtection: true }))
+}
+
+window.pingWithPIN = function () {
+  log(`ping ${++pingCount} with PIN`, manager.exec('ping', { message: 'ping' + pingCount, pinProtection: true }))
+}
+
+window.ping = function () {
+  log(`ping ${++pingCount}`, manager.exec('ping', { message: 'ping' + pingCount }))
+}
+
+window.cancelPending = function () {
+  log('cancelPending', window.keepkey.cancel())
+}

src/device.ts

@@ -84,6 +84,8 @@ export default abstract class Device {
 
   public abstract sendRaw (buffer: ByteBuffer): Promise<ByteBuffer>
 
+  public abstract cancelPending (): Promise<void>
+
   protected toMessageBuffer (msgTypeEnum: number, msg: jspb.Message): ByteBuffer {
     const messageBuffer = msg.serializeBinary()
 
@@ -112,4 +114,15 @@ export default abstract class Device {
     const reader = new jspb.BinaryReader(dataView.slice(9), 0, buff.limit - (9 + 2))
     return [typeID, MessageType.deserializeBinaryFromReader(msg, reader)]
   }
+
+  protected static failureMessageFactory (e?: Error | string) {
+    const msg = new Messages.Failure()
+    msg.setCode(Types.FailureType.FAILURE_UNEXPECTEDMESSAGE)
+    if (typeof e === 'string') {
+      msg.setMessage(e)
+    } else {
+      msg.setMessage(String(e))
+    }
+    return ByteBuffer.wrap(msg.serializeBinary())
+  }
 }

src/keepkey.ts

@@ -138,11 +138,7 @@ export default class KeepKey {
   // Cancel aborts the last device action that required user interaction
   // It can follow a button request, passphrase request, or pin request
   public async cancel (): Promise<void> {
-    const cancel = new Messages.Cancel()
-    // send
-    await this.device.exchange(Messages.MessageType.MESSAGETYPE_CANCEL, cancel)
-
-    // Emit event to notify clients that an action has been cancelled
+    await this.device.cancelPending()
     this.device.events.emit('CANCEL_ACTION')
   }
 

src/webUSBDevice.test.ts

@@ -7,7 +7,7 @@ describe('WebUSBDevice', () => {
   test('should return a Failure message if `read` throws an error', async () => {
     const config = { usbDevice: {
       transferIn: jest.fn().mockRejectedValueOnce(new Error('TEST'))
-    }, events: {} }
+    }, events: { emit: jest.fn() } }
     // @ts-ignore
     const device = new WebUSBDevice(config)
     const msg = new Messages.Cancel()

src/webUSBDevice.ts

@@ -4,7 +4,6 @@ import { default as PQueue } from 'p-queue'
 
 import Device from './device'
 import Messages from './kkProto/messages_pb'
-import Types from './kkProto/types_pb'
 
 export interface WebUSBDeviceConfig {
   usbDevice: USBDevice,
@@ -43,13 +42,32 @@ export default class WebUSBDevice extends Device {
     }
   }
 
+  public async cancelPending () {
+    console.log('pending', this.queue.pending)
+    try {
+      // If there are no pending commands, we should wait for a read back from the cancel command
+      // Otherwise the pending promise will read the error
+      if (this.queue.pending === 0) {
+        this.queue.add(() => this.read(), { priority: 1000 })
+          .then(() => console.log('cancenPending read done'))
+          .catch(e => console.log('cancenPending read failed', e))
+      }
+
+      const cancelMsg = new Messages.Cancel()
+      const buffer = this.toMessageBuffer(Messages.MessageType.MESSAGETYPE_CANCEL, cancelMsg)
+      await this.write(buffer)
+    } catch (e) {
+      console.error('Cancel Pending Error', e)
+    }
+  }
+
   public async disconnect (): Promise<void> {
     if (!this.usbDevice.opened) return
     try {
       // If the device is disconnected, this will fail and throw, which is fine.
       await this.usbDevice.releaseInterface(0)
     } catch (e) {
-      console.log(e)
+      console.log('Disconnect Error (Ignored):', e)
     }
   }
 
@@ -66,16 +84,14 @@ export default class WebUSBDevice extends Device {
         await this.write(buffer)
         return await this.read()
       } catch (e) {
-        const msg = new Messages.Failure()
-        msg.setCode(Types.FailureType.FAILURE_UNEXPECTEDMESSAGE)
-        msg.setMessage(String(e))
-        return ByteBuffer.wrap(msg.serializeBinary())
+        return Device.failureMessageFactory(e)
       }
     })
   }
 
   protected async write (buff: ByteBuffer): Promise<void> {
     // break frame into segments
+    this.events.emit('write', buff)
     for (let i = 0; i < buff.limit; i += SEGMENT_SIZE) {
       let segment = buff.toArrayBuffer().slice(i, i + SEGMENT_SIZE)
       let padding = new Array(SEGMENT_SIZE - segment.byteLength + 1).join('\0')
@@ -89,6 +105,7 @@ export default class WebUSBDevice extends Device {
   }
 
   protected async read (): Promise<ByteBuffer> {
+    this.events.emit('reading')
     let first = await this.readChunk()
     // Check that buffer starts with: "?##" [ 0x3f, 0x23, 0x23 ]
     // "?" = USB marker, "##" = KeepKey magic bytes
@@ -112,7 +129,9 @@ export default class WebUSBDevice extends Device {
         }
       }
 
-      return ByteBuffer.wrap(buffer)
+      const res = ByteBuffer.wrap(buffer)
+      this.events.emit('read', res)
+      return res
     } else {
       console.error('Invalid message', { msgLength, valid, first })
       throw new Error('Invalid message')