From 6102ff50a5372fcaa37f705fcff4352bc62bcb2f Mon Sep 17 00:00:00 2001 From: Shinrai Date: Sat, 3 Oct 2026 17:33:40 -0700 Subject: [PATCH 1/2] fix(events): give each remote its own emitter and never throw on an unheard error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All remotes shared one module-level EventEmitter, so with two TVs each remote's listeners received the other's connect, log and error events, and off() on one affected the other (#43). The emitter and the emitLog / emitError / handleDisconnectError helpers are now created per remote by createEventChannel() inside createRemote(). An `error` event with no listener made EventEmitter throw, crashing the consumer on the first connection or protocol error — including a failed auto-connect inside createRemote(), before a listener could be attached (#44). emitError now emits `error` only when a listener exists; otherwise it emits a `log` event with level "error" (the Error in data.error) and writes to NODE_DEBUG=android-tv-remote. The failing operation still surfaces the error: commands reject, connect()/disconnect() resolve with the Error as before, and initPromise rejects. initPromise also gets a no-op handler so a failed auto-connect isn't an unhandled rejection for callers who never await it. Tests mock @devicefarmer/adbkit (no ADB, device or socket): two remotes are independent for internal events, custom emits and off(); errors with and without a listener for connect, keyboard.key and auto-connect. Fixes #43 Fixes #44 --- README.md | 2 +- src/lib/android-tv-remote.mjs | 221 +++++++++++++++++++--------------- tests/events.test.vitest.mjs | 131 ++++++++++++++++++++ 3 files changed, 259 insertions(+), 95 deletions(-) create mode 100644 tests/events.test.vitest.mjs diff --git a/README.md b/README.md index 1ae2338..e05f92f 100644 --- a/README.md +++ b/README.md @@ -351,7 +351,7 @@ await remote.keyboard.key.a.keycode(); // Sends keycode instead of character ### Events - `log` - Emitted for all operations (info, warn, error, debug levels) -- `error` - Emitted when errors occur (structured error data) +- `error` - Emitted when errors occur (structured error data). Each remote has its own listeners. With no `error` listener attached, the error is emitted as a `log` event with `level: 'error'` (the `Error` is in `data.error`) instead of throwing. The failing call still reports it: commands reject, and `connect()` / `disconnect()` resolve with the `Error` as before - `screencap-start` - Emitted when screenshot capture begins - `screencap-captured` - Emitted when raw screenshot is captured - `screencap-processing` - Emitted when image processing begins diff --git a/src/lib/android-tv-remote.mjs b/src/lib/android-tv-remote.mjs index c4178a9..cb6052c 100644 --- a/src/lib/android-tv-remote.mjs +++ b/src/lib/android-tv-remote.mjs @@ -142,6 +142,7 @@ import adbkit from "@devicefarmer/adbkit"; import { EventEmitter } from "events"; +import { debuglog } from "node:util"; import { createWriteStream } from "fs"; import sharp from "sharp"; // @devicefarmer/adbkit is plain CJS (`exports.default` / `exports.Adb` both @@ -170,114 +171,141 @@ function wrapAsync(fn) { }; } -// Create event emitter for this instance -const emitter = new EventEmitter(); - /** - * Emit a log event with structured data. + * Debug channel for errors no "error" listener is attached to. + * Enable with NODE_DEBUG=android-tv-remote. * @private - * @param {string} level - Log level (info, warn, error, debug). - * @param {string} message - Log message. - * @param {string} [source] - Source of the log message. - * @param {any} [data] - Additional data to include. */ -function emitLog(level, message, source = "android-tv-remote", data = null) { - emitter.emit("log", { - level, - message, - source, - timestamp: new Date().toISOString(), - ...(data && { data }) - }); -} +const debug = debuglog("android-tv-remote"); /** - * Emit an error event with structured data. + * Create the event channel for one remote instance. Every remote gets its own + * EventEmitter, so listeners on one remote never see another remote's events. * @private - * @param {Error} error - The error object. - * @param {string} [source] - Source of the error. - * @param {string} [message] - Additional error message. + * @returns {{ emitter: EventEmitter, emitLog: Function, emitError: Function, handleDisconnectError: Function }} */ -function emitError(error, source = "android-tv-remote", message = null) { - // Filter out libspng/PNG processing errors that occur after disconnection - // These are common when background operations try to process data after disconnect - const errorMsg = error.message || ""; - if (errorMsg.includes("libspng") || errorMsg.includes("pngload_buffer") || errorMsg.includes("read error")) { - // Log as debug instead of error to avoid noise - emitLog("debug", `PNG processing error (likely post-disconnect): ${errorMsg}`, source); - return; - } +function createEventChannel() { + const emitter = new EventEmitter(); - emitter.emit("error", { - error, - source, - message: message || error.message, - timestamp: new Date().toISOString() - }); -} + /** + * Emit a log event with structured data. + * @private + * @param {string} level - Log level (info, warn, error, debug). + * @param {string} message - Log message. + * @param {string} [source] - Source of the log message. + * @param {any} [data] - Additional data to include. + */ + function emitLog(level, message, source = "android-tv-remote", data = null) { + emitter.emit("log", { + level, + message, + source, + timestamp: new Date().toISOString(), + ...(data && { data }) + }); + } -/** - * Handles disconnect and connection errors, emits helpful messages. - * Also provides onboarding steps for common authentication and connection issues. - * @private - * @param {Error} err - The error object. - * @example - * try { - * // ...code that may throw - * } catch (err) { - * handleDisconnectError(err); - * } - */ + /** + * Emit an error event with structured data. + * @private + * @param {Error} error - The error object. + * @param {string} [source] - Source of the error. + * @param {string} [message] - Additional error message. + */ + function emitError(error, source = "android-tv-remote", message = null) { + // Filter out libspng/PNG processing errors that occur after disconnection + // These are common when background operations try to process data after disconnect + const errorMsg = error.message || ""; + if (errorMsg.includes("libspng") || errorMsg.includes("pngload_buffer") || errorMsg.includes("read error")) { + // Log as debug instead of error to avoid noise + emitLog("debug", `PNG processing error (likely post-disconnect): ${errorMsg}`, source); + return; + } -function handleDisconnectError(err) { - emitError(err, "handleDisconnectError"); - - if (err.message && (err.message.includes("device unauthorized") || err.message.includes("failed to authenticate"))) { - emitError(err, "handleDisconnectError", "Device unauthorized - authentication required"); - emitLog( - "error", - "Your device is unauthorized or failed to authenticate. Please check your TV and accept the authorization dialog to allow this system to connect via ADB.", - "handleDisconnectError" - ); - emitLog( - "info", - "If you do not see a prompt, try disconnecting and reconnecting the device, or reboot your TV.", - "handleDisconnectError" - ); - emitLog( - "info", - "If the problem persists, remove the device from the list of authorized ADB devices in Developer Options and try again.", - "handleDisconnectError" - ); - emitLog( - "info", - "Tip: In Developer Options on your TV, try toggling 'ADB Debugging' off and then back on. This often resolves authentication issues.", - "handleDisconnectError" - ); + const payload = { + error, + source, + message: message || error.message, + timestamp: new Date().toISOString() + }; + + // EventEmitter throws when "error" is emitted with no listener, which would crash + // the consumer's process. Without a listener, route the error to the log channel + // (and NODE_DEBUG=android-tv-remote) instead; the failing operation still rejects + // or resolves with the error, so it is never silently lost. + if (emitter.listenerCount("error") > 0) { + emitter.emit("error", payload); + return; + } + debug("unhandled error from %s: %s", source, payload.message); + emitLog("error", payload.message, source, { error }); } - if (err.message && (err.message.includes("actively refused") || err.message.includes("No connection could be made"))) { - emitError(err, "handleDisconnectError", "Connection refused - ADB not enabled"); - emitLog( - "error", - "The device refused the connection. To enable ADB, follow these steps on your Android TV or Fire TV:", - "handleDisconnectError" - ); - emitLog("info", "1. Open Settings > Device Preferences > About (or My Fire TV > About)", "handleDisconnectError"); - emitLog("info", "2. Scroll to 'Build' and press OK 7 times to enable Developer Options", "handleDisconnectError"); - emitLog("info", "3. Go back to Settings > Device Preferences > Developer Options", "handleDisconnectError"); - emitLog( - "info", - "4. Enable 'Developer Options' if needed, then enable 'ADB Debugging' and 'Apps from Unknown Sources'", - "handleDisconnectError" - ); - emitLog("info", "5. Ensure your TV and computer are on the same network", "handleDisconnectError"); - emitLog("info", "6. On your computer, run: adb connect :5555", "handleDisconnectError"); - emitLog("info", "7. Accept the authorization prompt on your TV", "handleDisconnectError"); - emitLog("info", "If you do not see 'Developer Options', repeat step 2 until it appears.", "handleDisconnectError"); + /** + * Handles disconnect and connection errors, emits helpful messages. + * Also provides onboarding steps for common authentication and connection issues. + * @private + * @param {Error} err - The error object. + * @example + * try { + * // ...code that may throw + * } catch (err) { + * handleDisconnectError(err); + * } + */ + + function handleDisconnectError(err) { + emitError(err, "handleDisconnectError"); + + if (err.message && (err.message.includes("device unauthorized") || err.message.includes("failed to authenticate"))) { + emitError(err, "handleDisconnectError", "Device unauthorized - authentication required"); + emitLog( + "error", + "Your device is unauthorized or failed to authenticate. Please check your TV and accept the authorization dialog to allow this system to connect via ADB.", + "handleDisconnectError" + ); + emitLog( + "info", + "If you do not see a prompt, try disconnecting and reconnecting the device, or reboot your TV.", + "handleDisconnectError" + ); + emitLog( + "info", + "If the problem persists, remove the device from the list of authorized ADB devices in Developer Options and try again.", + "handleDisconnectError" + ); + emitLog( + "info", + "Tip: In Developer Options on your TV, try toggling 'ADB Debugging' off and then back on. This often resolves authentication issues.", + "handleDisconnectError" + ); + } + + if (err.message && (err.message.includes("actively refused") || err.message.includes("No connection could be made"))) { + emitError(err, "handleDisconnectError", "Connection refused - ADB not enabled"); + emitLog( + "error", + "The device refused the connection. To enable ADB, follow these steps on your Android TV or Fire TV:", + "handleDisconnectError" + ); + emitLog("info", "1. Open Settings > Device Preferences > About (or My Fire TV > About)", "handleDisconnectError"); + emitLog("info", "2. Scroll to 'Build' and press OK 7 times to enable Developer Options", "handleDisconnectError"); + emitLog("info", "3. Go back to Settings > Device Preferences > Developer Options", "handleDisconnectError"); + emitLog( + "info", + "4. Enable 'Developer Options' if needed, then enable 'ADB Debugging' and 'Apps from Unknown Sources'", + "handleDisconnectError" + ); + emitLog("info", "5. Ensure your TV and computer are on the same network", "handleDisconnectError"); + emitLog("info", "6. On your computer, run: adb connect :5555", "handleDisconnectError"); + emitLog("info", "7. Accept the authorization prompt on your TV", "handleDisconnectError"); + emitLog("info", "If you do not see 'Developer Options', repeat step 2 until it appears.", "handleDisconnectError"); + } + + return err; } - return err; + return { emitter, emitLog, emitError, handleDisconnectError }; } /** @@ -358,6 +386,8 @@ export default async function createRemote(config) { timeout: connectTimeout }); const device = client.getDevice(host); + // Per-instance event channel: listeners are scoped to this remote. + const { emitter, emitLog, emitError, handleDisconnectError } = createEventChannel(); let connected = false; let backgroundOperations = new Set(); @@ -454,6 +484,9 @@ export default async function createRemote(config) { }, INIT_TIMEOUT_MS); }) ]); + // Callers who never look at initPromise must not get an unhandled rejection (which + // crashes Node.js) when auto-connect fails; awaiting it still rejects as before. + initPromise.catch(() => {}); /** * Resets the disconnect timer if autoDisconnect is enabled. diff --git a/tests/events.test.vitest.mjs b/tests/events.test.vitest.mjs new file mode 100644 index 0000000..1cf7d34 --- /dev/null +++ b/tests/events.test.vitest.mjs @@ -0,0 +1,131 @@ +/** + * Event behaviour of the remote: every remote has its own event emitter (#43), + * and an `error` event with no listener never crashes the process — it goes to + * the `log` channel and the pending promise instead (#44). + * + * @devicefarmer/adbkit is mocked, so no ADB server, device or socket is used. + */ + +import { describe, test, expect, vi, beforeEach } from "vitest"; + +const client = { + connect: vi.fn(), + disconnect: vi.fn(), + listDevices: vi.fn(), + getDevice: vi.fn(() => ({ shell: vi.fn(async () => "") })) +}; + +vi.mock("@devicefarmer/adbkit", () => { + const Adb = { createClient: () => client, util: { readAll: async (x) => Buffer.from(String(x)) } }; + return { default: { Adb }, Adb }; +}); + +const { default: createRemote } = await import("../src/lib/android-tv-remote.mjs"); + +/** Config for an offline remote: no auto-connect, no heartbeat timers. */ +const offline = (extra = {}) => ({ ip: "10.0.0.1", autoConnect: false, maintainConnection: false, quiet: false, ...extra }); + +beforeEach(() => { + client.connect.mockReset().mockResolvedValue(true); + client.disconnect.mockReset().mockResolvedValue(true); + client.listDevices.mockReset().mockResolvedValue([]); +}); + +describe("per-instance event emitter (#43)", () => { + test("a listener on one remote doesn't receive another remote's events", async () => { + const a = await createRemote(offline({ ip: "10.0.0.1" })); + const b = await createRemote(offline({ ip: "10.0.0.2" })); + const onA = vi.fn(); + const onB = vi.fn(); + a.on("log", onA); + b.on("log", onB); + + await a.connect(); + + expect(onA).toHaveBeenCalledWith(expect.objectContaining({ level: "info", message: "Connected to 10.0.0.1:5555", source: "connect" })); + expect(onB).not.toHaveBeenCalled(); + + a.emit("custom", 1); + const onCustomB = vi.fn(); + b.on("custom", onCustomB); + a.emit("custom", 2); + expect(onCustomB).not.toHaveBeenCalled(); + }); + + test("error events stay on the remote that failed", async () => { + const a = await createRemote(offline({ ip: "10.0.0.1" })); + const b = await createRemote(offline({ ip: "10.0.0.2" })); + const errA = vi.fn(); + const errB = vi.fn(); + a.on("error", errA); + b.on("error", errB); + + await expect(a.keyboard.key("no-such-key")).rejects.toThrow("Unknown keyboard key: no-such-key"); + + expect(errA).toHaveBeenCalledTimes(1); + expect(errB).not.toHaveBeenCalled(); + }); + + test("removing a listener on one remote leaves the other remote's listener in place", async () => { + const a = await createRemote(offline({ ip: "10.0.0.1" })); + const b = await createRemote(offline({ ip: "10.0.0.2" })); + const listener = vi.fn(); + a.on("ping", listener); + b.on("ping", listener); + + a.off("ping", listener); + a.emit("ping"); + b.emit("ping"); + + expect(listener).toHaveBeenCalledTimes(1); + }); +}); + +describe("error events without a listener (#44)", () => { + test("a connection error with no error listener doesn't throw and goes to the log channel", async () => { + client.connect.mockRejectedValue(new Error("connection refused")); + const remote = await createRemote(offline()); + const onLog = vi.fn(); + remote.on("log", onLog); + + const result = await remote.connect(); + + expect(result).toBeInstanceOf(Error); + expect(remote.isConnected).toBe(false); + expect(onLog).toHaveBeenCalledWith( + expect.objectContaining({ level: "error", message: "connection refused", source: "handleDisconnectError", data: { error: result } }) + ); + }); + + test("a connection error is delivered to an attached error listener", async () => { + const failure = new Error("connection refused"); + client.connect.mockRejectedValue(failure); + const remote = await createRemote(offline()); + const onError = vi.fn(); + const onLog = vi.fn(); + remote.on("error", onError); + remote.on("log", onLog); + + await remote.connect(); + + expect(onError).toHaveBeenCalledWith( + expect.objectContaining({ error: failure, source: "handleDisconnectError", message: "connection refused" }) + ); + expect(onLog).not.toHaveBeenCalledWith(expect.objectContaining({ level: "error", source: "handleDisconnectError" })); + }); + + test("an operation error with no listener rejects the operation's promise instead of throwing", async () => { + const remote = await createRemote(offline()); + + await expect(remote.keyboard.key("no-such-key")).rejects.toThrow("Unknown keyboard key: no-such-key"); + }); + + test("a failed auto-connect with no listener resolves the remote and rejects initPromise", async () => { + client.connect.mockRejectedValue(new Error("connection refused")); + + const remote = await createRemote({ ip: "10.0.0.1", maintainConnection: false }); + + expect(remote.isConnected).toBe(false); + await expect(remote.initPromise).rejects.toThrow("Failed to connect to device on initialization."); + }); +}); From 49f4341a399f5f9de4f27f772a7f92356634824b Mon Sep 17 00:00:00 2001 From: Shinrai Date: Sat, 3 Oct 2026 17:34:50 -0700 Subject: [PATCH 2/2] chore: stamp file headers on the new test files --- tests/events.test.vitest.mjs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/events.test.vitest.mjs b/tests/events.test.vitest.mjs index 1cf7d34..c2ea603 100644 --- a/tests/events.test.vitest.mjs +++ b/tests/events.test.vitest.mjs @@ -1,3 +1,18 @@ +/** + * + * @Project: @cldmv/node-android-tv-remote + * @Filename: /tests/events.test.vitest.mjs + * @Date: 2026-10-03T17:33:40-07:00 (1791074020) + * @Author: Nate Corcoran + * @Email: + * ----- + * @Last modified by: Nate Corcoran (Shinrai@users.noreply.github.com) + * @Last modified time: 2026-10-03T17:34:50-07:00 (1791074090) + * ----- + * @Copyright: Copyright (c) 2013-2026 Catalyzed Motivation Inc. All rights reserved. + * + */ + /** * Event behaviour of the remote: every remote has its own event emitter (#43), * and an `error` event with no listener never crashes the process — it goes to