fix: address CodeRabbit review comments

- Add missing newline at EOF in .github/workflows/publish.yml
- Replace console.log with proper error handling in buttons.ts
- Improve error handling in Zemu.ts close method with try-catch-finally
- Add setScrambleKey support to transport proxy for comprehensive error tracking
- Remove debugging console.log statements from tests
- Remove skipped test that was only for exploration
- Replace console.warn with process.stderr.write for cleaner logging

Author: jleni

.github/workflows/publish.yml

@@ -25,4 +25,5 @@ jobs:
       timeout_minutes: 10
       dry_run: false
     secrets:
-      NPM_TOKEN: ${{ secrets.NPM_TOKEN_PUBLISH_AUTO }}
+      NPM_TOKEN: ${{ secrets.NPM_TOKEN_PUBLISH_AUTO }}
+

src/Zemu.ts

@@ -191,13 +191,15 @@ export default class Zemu {
 
   static stopAllEmuContainers(): void {
     const timer = setTimeout(() => {
-      console.log('Could not kill all containers before timeout!')
+      process.stderr.write('Could not kill all containers before timeout!\n')
       process.exit(1)
     }, KILL_TIMEOUT)
 
     // Clean up pool first
     if (Zemu.containerPool) {
-      Zemu.containerPool.cleanup().catch((error) => console.warn('Failed to cleanup container pool:', error))
+      Zemu.containerPool.cleanup().catch((error) => {
+        process.stderr.write(`Failed to cleanup container pool: ${error}\n`)
+      })
     }
 
     // Then kill any remaining containers
@@ -409,31 +411,32 @@ export default class Zemu {
   }
 
   async close(): Promise<void> {
-    try {
-      this.stopGRPCServer()
+    this.stopGRPCServer()
 
+    try {
       if (this.usingPool && this.pooledContainer && Zemu.containerPool) {
         this.log('Returning container to pool')
         await Zemu.containerPool.release(this.pooledContainer)
-        this.usingPool = false
-        this.pooledContainer = null
       } else {
         this.log('Stopping container')
         await this.emuContainer.stop()
       }
     } catch (error) {
       this.log(`Error during close: ${error}`)
       // If pool return fails, try to stop container directly
+      if (this.usingPool && this.emuContainer) {
+        this.log('Attempting direct container stop after pool release failure')
+        await this.emuContainer.stop().catch((stopError) => {
+          this.log(`Failed to stop container directly: ${stopError}`)
+        })
+      }
+      throw error
+    } finally {
+      // Always clean up state
       if (this.usingPool) {
-        try {
-          await this.emuContainer.stop()
-        } catch (stopError) {
-          this.log(`Failed to stop container after pool release failure: ${stopError}`)
-        }
         this.usingPool = false
         this.pooledContainer = null
       }
-      throw error
     }
   }
 
@@ -469,7 +472,7 @@ export default class Zemu {
           }
         }
 
-        // For exchange and other methods, apply similar wrapping
+        // For exchange method, apply similar wrapping
         if (prop === 'exchange') {
           return async function (apdu: Buffer) {
             try {
@@ -487,6 +490,23 @@ export default class Zemu {
           }
         }
 
+        // For setScrambleKey method, wrap if it exists
+        if (prop === 'setScrambleKey' && typeof target[prop] === 'function') {
+          return async function (key: string) {
+            try {
+              self.lastTransportError = null
+              const result = await (target as any).setScrambleKey(key)
+              return result
+            } catch (error) {
+              self.lastTransportError = error as Error
+              if (isCriticalTransportError(error)) {
+                self.log(`Critical transport error in setScrambleKey: ${error}`)
+              }
+              throw error
+            }
+          }
+        }
+
         // For all other properties/methods, return as-is
         return Reflect.get(target, prop, receiver)
       },

src/buttons.ts

@@ -18,7 +18,7 @@ import { flex } from './buttons_flex'
 import { stax } from './buttons_stax'
 import { type ButtonKind, type IButton, SwipeDirection, type TModel } from './types'
 
-const dummyButton: IButton = {
+const _dummyButton: IButton = {
   x: 0,
   y: 0,
   delay: 0,
@@ -32,23 +32,20 @@ export function getTouchElement(model: TModel, buttonKind: ButtonKind): IButton
       if (button != null) {
         return button
       }
-      break
+      throw new Error(`ButtonKind ${buttonKind} not found for model ${model}`)
     }
 
     case 'flex': {
       const button = flex.TouchElements.get(buttonKind)
       if (button != null) {
         return button
       }
-      break
+      throw new Error(`ButtonKind ${buttonKind} not found for model ${model}`)
     }
 
     // Add cases for other models here when they become available
 
     default:
-      return dummyButton
+      throw new Error(`Unsupported ButtonKind: ${model}, ${buttonKind}`)
   }
-
-  console.log(`Unsupported ButtonKind: ${model}, ${buttonKind}`)
-  return dummyButton
 }

src/emulator.ts

@@ -58,7 +58,6 @@ export default class EmuContainer {
     docker.listContainers({ all: true, filters: { name: [name] } }, (listError, containers?: ContainerInfo[]) => {
       if (listError != null) throw listError
       if (containers == null || containers.length === 0) {
-        console.log('No containers found')
         return
       }
       for (const containerInfo of containers) {

tests/error-handling-simple.test.ts

@@ -16,35 +16,6 @@ const ZEMU_OPTIONS_S: IStartOptions = {
 }
 
 describe('Error Handling - Simple', () => {
-  test.skip('First understand current timeout behavior', async () => {
-    const sim = new Zemu(DEMO_APP_PATH_S)
-
-    try {
-      await sim.start(ZEMU_OPTIONS_S)
-      const transport = sim.getTransport()
-
-      console.log('=== Testing invalid APDU command ===')
-
-      // Send an invalid APDU command that should trigger error 0x6984
-      const startTime = Date.now()
-
-      try {
-        // Invalid CLA (0xFF) should cause error
-        const result = await transport.send(0xff, 0x00, 0x00, 0x00)
-        console.log('Result:', result)
-      } catch (error: any) {
-        const elapsedTime = Date.now() - startTime
-        console.log(`Error occurred after ${elapsedTime}ms`)
-        console.log('Error:', error)
-        console.log('Error statusCode:', error.statusCode)
-        console.log('Error name:', error.name)
-        console.log('Error message:', error.message)
-      }
-    } finally {
-      await sim.close()
-    }
-  }, 30000)
-
   test('Test waitUntilScreenIs with timeout', async () => {
     // Disable pooling globally for this test
     process.env.ZEMU_CONTAINER_POOLING = 'false'
@@ -53,7 +24,6 @@ describe('Error Handling - Simple', () => {
 
     try {
       await sim.start(ZEMU_OPTIONS_S)
-      console.log('=== Testing waitUntilScreenIs behavior ===')
 
       // Get a snapshot that will never match to force timeout
       const snapshot = await sim.snapshot()
@@ -71,8 +41,6 @@ describe('Error Handling - Simple', () => {
         await sim.waitUntilScreenIs(modifiedSnapshot, 2000)
       } catch (error: any) {
         const elapsedTime = Date.now() - startTime
-        console.log(`Timeout occurred after ${elapsedTime}ms`)
-        console.log('Error message:', error.message)
 
         // Verify it actually waited for the timeout
         expect(elapsedTime).toBeGreaterThan(1900)