diff --git a/package-lock.json b/package-lock.json index 87a88938..212046fe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26,7 +26,7 @@ "postman-request": "^2.88.1-postman.48", "replace-in-file": "^6.3.2", "replace-last": "^1.2.6", - "roku-deploy": "^3.18.2", + "roku-deploy": "^4.0.0-alpha.3", "semver": "^7.5.4", "serialize-error": "^8.1.0", "smart-buffer": "^4.2.0", @@ -1603,6 +1603,43 @@ "graceful-fs": "^4.1.6" } }, + "node_modules/brighterscript/node_modules/roku-deploy": { + "version": "3.18.2", + "resolved": "https://registry.npmjs.org/roku-deploy/-/roku-deploy-3.18.2.tgz", + "integrity": "sha512-O0nO/hzhax6nhcwsk40Vdoq7RKm4n3OCJfc+Vr5OcV1YfSg5qeFt6gY3RG74QjZ+KrTO1A51XO0Oe1e4qV4O8w==", + "license": "MIT", + "dependencies": { + "@rokucommunity/logger": "^0.4.1", + "chalk": "^2.4.2", + "fast-glob": "^3.2.12", + "fs-extra": "^7.0.1", + "is-glob": "^4.0.3", + "jsonc-parser": "^2.3.0", + "jszip": "^3.6.0", + "micromatch": "^4.0.4", + "needle": "^3.5.0", + "picomatch": "^2.3.2", + "semver": "^7.7.3", + "xml2js": "^0.5.0" + }, + "bin": { + "roku-deploy": "dist/cli.js" + } + }, + "node_modules/brighterscript/node_modules/roku-deploy/node_modules/fs-extra": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-7.0.1.tgz", + "integrity": "sha512-YJDaCJZEnBmcbw13fvdAM9AwNOJwOzrE4pqMqBq5nFiEqXUqHwlK4B+3pUw6JNvfSPtX05xFHtYy/1ni01eGCw==", + "license": "MIT", + "dependencies": { + "graceful-fs": "^4.1.2", + "jsonfile": "^4.0.0", + "universalify": "^0.1.0" + }, + "engines": { + "node": ">=6 <7 || >=8" + } + }, "node_modules/brighterscript/node_modules/supports-color": { "version": "5.5.0", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.5.0.tgz", @@ -4710,9 +4747,9 @@ } }, "node_modules/roku-deploy": { - "version": "3.18.2", - "resolved": "https://registry.npmjs.org/roku-deploy/-/roku-deploy-3.18.2.tgz", - "integrity": "sha512-O0nO/hzhax6nhcwsk40Vdoq7RKm4n3OCJfc+Vr5OcV1YfSg5qeFt6gY3RG74QjZ+KrTO1A51XO0Oe1e4qV4O8w==", + "version": "4.0.0-alpha.3", + "resolved": "https://registry.npmjs.org/roku-deploy/-/roku-deploy-4.0.0-alpha.3.tgz", + "integrity": "sha512-WpQSPD5m1cuxV80aSiDtu5oVAS1V2ouKGHDusKk/FWJznBrPReGk9TDkZZ2BDo3iK3wqmxRpC918DgXQ1lFVtA==", "license": "MIT", "dependencies": { "@rokucommunity/logger": "^0.4.1", @@ -4721,12 +4758,14 @@ "fs-extra": "^7.0.1", "is-glob": "^4.0.3", "jsonc-parser": "^2.3.0", - "jszip": "^3.6.0", + "jszip": "^3.10.1", "micromatch": "^4.0.4", "needle": "^3.5.0", "picomatch": "^2.3.2", "semver": "^7.7.3", - "xml2js": "^0.5.0" + "ws": "^8.21.1", + "xml2js": "^0.5.0", + "yargs": "^17.7.2" }, "bin": { "roku-deploy": "dist/cli.js" @@ -4756,6 +4795,20 @@ "node": ">=4" } }, + "node_modules/roku-deploy/node_modules/cliui": { + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/cliui/-/cliui-8.0.1.tgz", + "integrity": "sha512-BSeNnyus75C4//NQ9gQt1/csTXyo/8Sb+afLAkzAptFuMsod9HFokGNudZpi/oQV73hnVK+sR+5PVRMd+Dr7YQ==", + "license": "ISC", + "dependencies": { + "string-width": "^4.2.0", + "strip-ansi": "^6.0.1", + "wrap-ansi": "^7.0.0" + }, + "engines": { + "node": ">=12" + } + }, "node_modules/roku-deploy/node_modules/color-convert": { "version": "1.9.3", "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-1.9.3.tgz", @@ -4825,6 +4878,24 @@ "node": ">= 4.0.0" } }, + "node_modules/roku-deploy/node_modules/yargs": { + "version": "17.7.3", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-17.7.3.tgz", + "integrity": "sha512-GZtjxm/J/4TSxuL3FNYjCmLktBTnIw/rVmKSIyKeYAZpmJB2ig9VauCC5xsa82GNKVKDAqpOn3KVzNt0zmrU0g==", + "license": "MIT", + "dependencies": { + "cliui": "^8.0.1", + "escalade": "^3.1.1", + "get-caller-file": "^2.0.5", + "require-directory": "^2.1.1", + "string-width": "^4.2.3", + "y18n": "^5.0.5", + "yargs-parser": "^21.1.1" + }, + "engines": { + "node": ">=12" + } + }, "node_modules/run-parallel": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/run-parallel/-/run-parallel-1.2.0.tgz", @@ -5795,9 +5866,9 @@ } }, "node_modules/ws": { - "version": "8.21.0", - "resolved": "https://registry.npmjs.org/ws/-/ws-8.21.0.tgz", - "integrity": "sha512-Vsp28b7DRcimFQvrqu2Wek3z1iYxDCWqHYB8Qsnk/S4RfaCQzPGPyBNuVjJV3cd6UiKtUtp6sNM77gWvzcCH+g==", + "version": "8.21.3", + "resolved": "https://registry.npmjs.org/ws/-/ws-8.21.3.tgz", + "integrity": "sha512-201TZ/kPWxoPr/OKWjquZR1SWKXcvxdH+e1xrx89b3YbmzLMFCLfnaG1HFIgWzJOEWZ7MvpK++odZufgYR50Rw==", "license": "MIT", "engines": { "node": ">=10.0.0" diff --git a/package.json b/package.json index cbe37f6f..ea2fabe2 100644 --- a/package.json +++ b/package.json @@ -119,7 +119,7 @@ "postman-request": "^2.88.1-postman.48", "replace-in-file": "^6.3.2", "replace-last": "^1.2.6", - "roku-deploy": "^3.18.2", + "roku-deploy": "^4.0.0-alpha.3", "semver": "^7.5.4", "serialize-error": "^8.1.0", "smart-buffer": "^4.2.0", diff --git a/src/Exceptions.ts b/src/Exceptions.ts index 8aa7dbaf..783fb553 100644 --- a/src/Exceptions.ts +++ b/src/Exceptions.ts @@ -8,5 +8,9 @@ export class SocketConnectionInUseError extends Error { } public port: number; + /** + * A label identifying the device the connection was made to: the host for a local device, or + * the instanceUrl/id/esn for a Roku Cloud Emulator device. + */ public host: string; } diff --git a/src/LaunchConfiguration.ts b/src/LaunchConfiguration.ts index 22464820..b21135b9 100644 --- a/src/LaunchConfiguration.ts +++ b/src/LaunchConfiguration.ts @@ -1,4 +1,4 @@ -import type { DeviceInfoRaw, FileEntry } from 'roku-deploy'; +import type { DeviceConfig, DeviceInfoRaw, FileEntry } from 'roku-deploy'; import type { DebugProtocol } from '@vscode/debugprotocol'; import type { LogLevel } from './logging'; @@ -12,8 +12,17 @@ export interface LaunchConfiguration extends DebugProtocol.LaunchRequestArgument cwd: string; /** * The host or ip address for the target Roku + * @deprecated Use `device` instead. When `device` is omitted, a local device config is built from this field. */ - host: string; + host?: string; + + /** + * The roku-deploy device config for the target device. This is the canonical way to address the + * device: a local network device (`{ host }`) or a Roku Cloud Emulator device + * (`{ instanceUrl | id | esn, rceToken }`). When omitted, a local device config is built from + * the deprecated `host` field. + */ + device?: DeviceConfig; /** * The raw `device-info` for the target Roku. When supplied, the debug session uses this instead of @@ -411,6 +420,17 @@ export interface LaunchConfiguration extends DebugProtocol.LaunchRequestArgument clientCapabilities?: ClientCapabilities; } +/** + * A launch configuration after the debug session has normalized it: the deprecated `host` field has + * been consumed (`normalizeLaunchConfig` converts it into `device`, the only time it is ever read, + * and then deletes it from the object) and `device` is a concrete roku-deploy device config. + * Everything inside the debugger works against this type; the raw `LaunchConfiguration` (with + * `host`) exists only at the DAP input boundary. + */ +export type ResolvedLaunchConfiguration = Omit & { + device: DeviceConfig; +}; + /** * Optional features the client advertises support for via `LaunchConfiguration.clientCapabilities`. * The debug adapter only enables the matching behavior when the client opts in here. diff --git a/src/PerfettoManager.spec.ts b/src/PerfettoManager.spec.ts index dbd889e6..6bd3179f 100644 --- a/src/PerfettoManager.spec.ts +++ b/src/PerfettoManager.spec.ts @@ -18,7 +18,7 @@ describe('PerfettoManager', () => { beforeEach(() => { perfettoManager = new PerfettoManager({ - host: '192.168.1.100', + device: { host: '192.168.1.100' }, enabled: true, dir: s`${tempDir}/profiling`, filename: 'test_${timestamp}.perfetto-trace', @@ -68,7 +68,7 @@ describe('PerfettoManager', () => { describe('constructor', () => { it('uses default values when not specified', () => { perfettoManager = new PerfettoManager({ - host: '192.168.1.100', + device: { host: '192.168.1.100' }, enabled: true, rootDir: rootDir }); @@ -81,15 +81,15 @@ describe('PerfettoManager', () => { it('uses provided values over defaults', () => { perfettoManager = new PerfettoManager({ - host: '10.0.0.1', + device: { host: '10.0.0.1' }, enabled: true, dir: '/custom/dir', channelId: 'prod', remotePort: 9090, rootDir: rootDir }); + expect((perfettoManager as any).config.device).to.eql({ host: '10.0.0.1' }); expect((perfettoManager as any).config).to.include({ - host: '10.0.0.1', dir: '/custom/dir', channelId: 'prod', remotePort: 9090 @@ -128,7 +128,7 @@ describe('PerfettoManager', () => { describe('startTracing', () => { it('throws when no host is configured', async () => { perfettoManager = new PerfettoManager({ - host: undefined as any, + device: undefined as any, enabled: true, rootDir: rootDir }); @@ -140,7 +140,7 @@ describe('PerfettoManager', () => { await perfettoManager.startTracing(); expect.fail('Should have thrown an error'); } catch (error) { - expect((error as Error).message).to.include('No host configured'); + expect((error as Error).message).to.include('Perfetto tracing requires a device with a host'); } // Should also emit error event @@ -345,7 +345,7 @@ describe('PerfettoManager', () => { sinon.stub(rokuECP, 'enablePerfettoTracing').rejects(new Error('No host configured')); perfettoManager = new PerfettoManager({ - host: undefined as any, + device: undefined as any, enabled: true, dir: '/tmp/traces' }); @@ -843,7 +843,7 @@ describe('PerfettoManager', () => { // Restore the stub to test actual createWebSocket sinon.restore(); perfettoManager = new PerfettoManager({ - host: '192.168.1.200', + device: { host: '192.168.1.200' }, remotePort: 8080, enabled: true, rootDir: rootDir diff --git a/src/PerfettoManager.ts b/src/PerfettoManager.ts index 206d29ac..f18a38fd 100644 --- a/src/PerfettoManager.ts +++ b/src/PerfettoManager.ts @@ -8,12 +8,19 @@ import { standardizePath as s } from 'brighterscript'; import { createLogger } from './logging'; import { rokuECP } from './RokuECP'; import { util } from './util'; +import { isLocalDeviceConfig } from 'roku-deploy'; +import type { DeviceConfig, LocalDeviceConfig } from 'roku-deploy'; /** * Configuration interface for Perfetto tracing */ interface PerfettoConfig { - host: string; + /** + * The roku-deploy device config for the target device. The perfetto trace WebSocket connects + * directly to the device's ECP port, so tracing currently requires a local device (one with a + * host); ECP commands route through roku-deploy and work for any device. + */ + device: DeviceConfig; enabled?: boolean; dir?: string; filename?: string; @@ -119,7 +126,8 @@ export class PerfettoManager { } private createWebSocket() { - const url = `ws://${this.config.host}:${this.config.remotePort}/perfetto-session`; + const device = this.config.device as LocalDeviceConfig; + const url = `ws://${device.host}:${this.config.remotePort}/perfetto-session`; this.socket = new WebSocket(url); return this.socket; } @@ -129,8 +137,11 @@ export class PerfettoManager { * @param includeResultOnStop whether to include the file path when the 'stop' event fires. This should be false if the caller is going to emit their own 'stop' event (like when heapSnapshot is the activator of tracing. */ public async startTracing(options?: { excludeResultOnStop: boolean }): Promise { - if (!this.config.host) { - throw this.emitError(new Error('No host configured for Perfetto tracing')); + //the trace websocket connects straight to the device's ECP port, so only host-addressed + //(local) devices are supported for now + const device = this.config.device; + if (!device || !isLocalDeviceConfig(device) || !device.host) { + throw this.emitError(new Error('Perfetto tracing requires a device with a host')); } try { @@ -293,11 +304,11 @@ export class PerfettoManager { * Enable tracing on the Roku device. This returns true if we were successful, and throws if we we failed to enable */ public async enableTracing(): Promise { - this.logger.log(`Enabling Perfetto tracing on channel ${this.config.channelId} at host ${this.config.host}`); + this.logger.log(`Enabling Perfetto tracing on channel ${this.config.channelId} on device ${util.getDeviceLabel(this.config.device)}`); try { const result = await rokuECP.enablePerfettoTracing({ - host: this.config.host, + device: this.config.device, remotePort: this.config.remotePort, channelId: this.config.channelId }); @@ -465,7 +476,7 @@ export class PerfettoManager { await rokuECP.captureHeapSnapshot({ channelId: this.config.channelId, - host: this.config.host, + device: this.config.device, remotePort: this.config.remotePort }); diff --git a/src/PerfettoManagerIntegration.spec.ts b/src/PerfettoManagerIntegration.spec.ts index 974eb967..05e0a88a 100644 --- a/src/PerfettoManagerIntegration.spec.ts +++ b/src/PerfettoManagerIntegration.spec.ts @@ -18,7 +18,7 @@ describe('Profiling/Tracing Integration Tests', () => { beforeEach(() => { perfettoManager = new PerfettoManager({ - host: '192.168.1.100', + device: { host: '192.168.1.100' }, enabled: true, dir: s`${tempDir}/profiling`, filename: 'test_${timestamp}.perfetto-trace', @@ -68,7 +68,7 @@ describe('Profiling/Tracing Integration Tests', () => { describe('TC-03: connectOnStart Behavior', () => { it('should create manager with connectOnStart: false by default', () => { const manager = new PerfettoManager({ - host: '192.168.1.100', + device: { host: '192.168.1.100' }, enabled: true }); // connectOnStart should not cause auto-start, just config storage @@ -77,7 +77,7 @@ describe('Profiling/Tracing Integration Tests', () => { it('should store connectOnStart: true in config', () => { const manager = new PerfettoManager({ - host: '192.168.1.100', + device: { host: '192.168.1.100' }, enabled: true, connectOnStart: true } as any); @@ -176,7 +176,7 @@ describe('Profiling/Tracing Integration Tests', () => { it('should generate unique filenames using sequence numbers', () => { // Create manager with sequence-based filename const manager = new PerfettoManager({ - host: '192.168.1.100', + device: { host: '192.168.1.100' }, enabled: true, dir: s`${tempDir}/profiling`, filename: 'test_${sequence}.perfetto-trace', @@ -276,21 +276,21 @@ describe('Profiling/Tracing Integration Tests', () => { describe('Configuration Defaults', () => { it('should use default channel ID "dev" when not specified', () => { const manager = new PerfettoManager({ - host: '192.168.1.100' + device: { host: '192.168.1.100' } }); expect((manager as any).config.channelId).to.equal('dev'); }); it('should use default port 8060 when not specified', () => { const manager = new PerfettoManager({ - host: '192.168.1.100' + device: { host: '192.168.1.100' } }); expect((manager as any).config.remotePort).to.equal(8060); }); it('should use default profiling directory when not specified', () => { const manager = new PerfettoManager({ - host: '192.168.1.100', + device: { host: '192.168.1.100' }, rootDir: '/app/root' }); expect((manager as any).config.dir).to.equal(s`/app/root/profiling`); @@ -298,14 +298,14 @@ describe('Profiling/Tracing Integration Tests', () => { it('should use custom values when provided', () => { const manager = new PerfettoManager({ - host: '10.0.0.1', + device: { host: '10.0.0.1' }, channelId: 'prod', remotePort: 9090, dir: '/custom/traces', rootDir: '/app' }); - expect((manager as any).config.host).to.equal('10.0.0.1'); + expect((manager as any).config.device).to.eql({ host: '10.0.0.1' }); expect((manager as any).config.channelId).to.equal('prod'); expect((manager as any).config.remotePort).to.equal(9090); expect((manager as any).config.dir).to.equal('/custom/traces'); diff --git a/src/RendezvousTracker.spec.ts b/src/RendezvousTracker.spec.ts index c4fa77ce..588c6cab 100644 --- a/src/RendezvousTracker.spec.ts +++ b/src/RendezvousTracker.spec.ts @@ -5,7 +5,7 @@ import type { RendezvousHistory } from './RendezvousTracker'; import { RendezvousTracker } from './RendezvousTracker'; import { SceneGraphDebugCommandController } from './SceneGraphDebugCommandController'; import type { LaunchConfiguration } from './LaunchConfiguration'; -import { util } from './util'; +import { rokuDeploy } from 'roku-deploy'; describe('BrightScriptFileUtils ', () => { let rendezvousTracker: RendezvousTracker; @@ -15,7 +15,7 @@ describe('BrightScriptFileUtils ', () => { beforeEach(() => { let launchConfig = { - 'host': '192.168.1.5', + 'device': { host: '192.168.1.5' }, 'remotePort': 8060 }; let deviceInfo = { @@ -390,32 +390,80 @@ describe('BrightScriptFileUtils ', () => { }); }); + describe('getEcpRendezvous', () => { + const rendezvousResult = { + trackingEnabled: true, + items: [{ + id: '1', + startTime: '100', + endTime: '200', + lineNumber: '10', + file: 'pkg:/components/MyComponent.brs' + }] + }; + + it('queries roku-deploy with the bare host when device is not set', async () => { + const queryRendezvousStub = sinon.stub(rokuDeploy, 'queryRendezvous').resolves(rendezvousResult); + + const result = await rendezvousTracker.getEcpRendezvous(); + + expect(queryRendezvousStub.getCall(0).args).to.eql([{ + device: { host: '192.168.1.5' }, + ecpPort: 8060 + }]); + expect(result).to.eql(rendezvousResult); + }); + + it('passes an RCE device config through to roku-deploy', async () => { + const rceDeviceConfig = { instanceUrl: 'https://device.rce.roku.com/instance/my-instance', rceToken: 'my-rce-token' }; + rendezvousTracker['launchConfiguration'].device = rceDeviceConfig; + const queryRendezvousStub = sinon.stub(rokuDeploy, 'queryRendezvous').resolves(rendezvousResult); + + const result = await rendezvousTracker.getEcpRendezvous(); + + expect(queryRendezvousStub.getCall(0).args[0].device).to.equal(rceDeviceConfig); + expect(result).to.eql(rendezvousResult); + }); + }); + describe('toggleEcpRendezvousTracking', () => { - async function doTest(statusCode: number, expectedValue: boolean) { - const stub = sinon.stub(util, 'httpPost').returns(Promise.resolve({ statusCode: statusCode } as any)); - expect( - await rendezvousTracker.toggleEcpRendezvousTracking('track') - ).to.eq(expectedValue); - expect( - await rendezvousTracker.toggleEcpRendezvousTracking('untrack') - ).to.eql(expectedValue); - stub.restore(); - } + it('returns true when roku-deploy succeeds', async () => { + const setRendezvousTrackingStub = sinon.stub(rokuDeploy, 'setRendezvousTracking').resolves(true); + + expect(await rendezvousTracker.toggleEcpRendezvousTracking('track')).to.be.true; + expect(setRendezvousTrackingStub.getCall(0).args).to.eql([{ + device: { host: '192.168.1.5' }, + enabled: true, + ecpPort: 8060 + }]); + + expect(await rendezvousTracker.toggleEcpRendezvousTracking('untrack')).to.be.true; + expect(setRendezvousTrackingStub.getCall(1).args).to.eql([{ + device: { host: '192.168.1.5' }, + enabled: false, + ecpPort: 8060 + }]); + }); + + it('returns false when roku-deploy throws', async () => { + sinon.stub(rokuDeploy, 'setRendezvousTracking').rejects(new Error('boom')); - it('returns true for 200 response code', async () => { - await doTest(200, true); - await doTest(202, true); - await doTest(205, true); - await doTest(299, true); + expect(await rendezvousTracker.toggleEcpRendezvousTracking('track')).to.be.false; + expect(await rendezvousTracker.toggleEcpRendezvousTracking('untrack')).to.be.false; }); - it('returns true for 200 response code', async () => { - await doTest(0, false); - await doTest(100, false); - await doTest(301, false); - await doTest(401, false); - await doTest(404, false); - await doTest(500, false); + it('passes an RCE device config through to roku-deploy', async () => { + const rceDeviceConfig = { instanceUrl: 'https://device.rce.roku.com/instance/my-instance', rceToken: 'my-rce-token' }; + rendezvousTracker['launchConfiguration'].device = rceDeviceConfig; + const setRendezvousTrackingStub = sinon.stub(rokuDeploy, 'setRendezvousTracking').resolves(true); + + expect(await rendezvousTracker.toggleEcpRendezvousTracking('track')).to.be.true; + expect(setRendezvousTrackingStub.getCall(0).args[0].device).to.equal(rceDeviceConfig); + expect(setRendezvousTrackingStub.getCall(0).args[0].enabled).to.equal(true); + + expect(await rendezvousTracker.toggleEcpRendezvousTracking('untrack')).to.be.true; + expect(setRendezvousTrackingStub.getCall(1).args[0].device).to.equal(rceDeviceConfig); + expect(setRendezvousTrackingStub.getCall(1).args[0].enabled).to.equal(false); }); }); diff --git a/src/RendezvousTracker.ts b/src/RendezvousTracker.ts index 94ba7143..d6f800e3 100644 --- a/src/RendezvousTracker.ts +++ b/src/RendezvousTracker.ts @@ -4,16 +4,15 @@ import * as replaceLast from 'replace-last'; import type { SourceLocation } from './managers/LocationManager'; import { logger } from './logging'; import { SceneGraphDebugCommandController } from './SceneGraphDebugCommandController'; -import * as xml2js from 'xml2js'; -import { util } from './util'; import * as semver from 'semver'; +import { rokuDeploy } from 'roku-deploy'; import type { DeviceInfo } from 'roku-deploy'; -import type { LaunchConfiguration } from './LaunchConfiguration'; +import type { ResolvedLaunchConfiguration } from './LaunchConfiguration'; export class RendezvousTracker { constructor( private deviceInfo: DeviceInfo, - private launchConfiguration: LaunchConfiguration + private launchConfiguration: ResolvedLaunchConfiguration ) { this.clientPathsMap = {}; this.emitter = new EventEmitter(); @@ -144,7 +143,7 @@ export class RendezvousTracker { * Run a SceneGraph logendezvous 8080 command and get the text output */ private async runSGLogrendezvousCommand(command: 'status' | 'on' | 'off'): Promise { - let sgDebugCommandController = new SceneGraphDebugCommandController(this.launchConfiguration.host, this.launchConfiguration.sceneGraphDebugCommandsPort); + let sgDebugCommandController = new SceneGraphDebugCommandController(this.launchConfiguration.device, this.launchConfiguration.sceneGraphDebugCommandsPort); try { this.logger.info(`port 8080 command: logrendezvous ${command}`); return (await sgDebugCommandController.logrendezvous(command)).result.rawResponse; @@ -202,40 +201,13 @@ export class RendezvousTracker { * Get the response from an ECP sgrendezvous request from the Roku */ public async getEcpRendezvous(): Promise { - const url = `http://${this.launchConfiguration.host}:${this.launchConfiguration.remotePort}/query/sgrendezvous`; - this.logger.trace(`Sending ECP rendezvous request:`, url); - // Send rendezvous query to ECP - const rendezvousQuery = await util.httpGet(url); - let rendezvousQueryData = rendezvousQuery.body as string; - let ecpData: EcpRendezvousData = { - trackingEnabled: false, - items: [] - }; - - this.logger.trace('Parsing rendezvous response', rendezvousQuery); - // Parse rendezvous query data - await new Promise((resolve, reject) => { - xml2js.parseString(rendezvousQueryData, (err, result) => { - if (err) { - reject(err); - } else { - const itemArray = result.sgrendezvous.data[0].item; - ecpData.trackingEnabled = result.sgrendezvous.data[0]['tracking-enabled'][0]; - if (Array.isArray(itemArray)) { - ecpData.items = itemArray.map((obj: any) => ({ - id: obj.id[0], - startTime: obj['start-tm'][0], - endTime: obj['end-tm'][0], - lineNumber: obj['line-number'][0], - file: obj.file[0] - })); - } - resolve(ecpData); - } - }); + this.logger.trace('Sending ECP rendezvous request'); + const rendezvous = await rokuDeploy.queryRendezvous({ + device: this.launchConfiguration.device, + ecpPort: this.launchConfiguration.remotePort }); - this.logger.trace('Parsed ECP rendezvous data:', ecpData); - return ecpData; + this.logger.trace('Parsed ECP rendezvous data:', rendezvous); + return rendezvous; } /** @@ -245,13 +217,12 @@ export class RendezvousTracker { public async toggleEcpRendezvousTracking(toggle: 'track' | 'untrack'): Promise { try { this.logger.log(`Sending ecp sgrendezvous request: ${toggle}`); - const response = await util.httpPost( - `http://${this.launchConfiguration.host}:${this.launchConfiguration.remotePort}/sgrendezvous/${toggle}`, - //not sure if we need this, but it works...so probably better to just leave it here - { body: '' } - ); - //this was successful if we got a 200 level status code (200-299) - return response.statusCode >= 200 && response.statusCode < 300; + await rokuDeploy.setRendezvousTracking({ + device: this.launchConfiguration.device, + enabled: toggle === 'track', + ecpPort: this.launchConfiguration.remotePort + }); + return true; } catch (e) { return false; } diff --git a/src/RokuECP.spec.ts b/src/RokuECP.spec.ts index 35b38a90..0a91be59 100644 --- a/src/RokuECP.spec.ts +++ b/src/RokuECP.spec.ts @@ -3,9 +3,10 @@ import { createSandbox } from 'sinon'; import { describe } from 'mocha'; import type { EcpAppStateData, EcpHeapSnapshotData, EcpRegistryData } from './RokuECP'; import { AppState, EcpStatus, rokuECP } from './RokuECP'; -import { util } from './util'; import { expectThrowsAsync } from './testHelpers.spec'; import { undent } from 'undent'; +import { rokuDeploy } from 'roku-deploy'; +import type { RokuAppState, RokuRegistry } from 'roku-deploy'; const sinon = createSandbox(); @@ -17,430 +18,300 @@ describe('RokuECP', () => { }); describe('doRequest', () => { - it('correctly builds url without leading /', async () => { + it('routes the request through rokuDeploy.sendEcpRequest as a GET by default', async () => { let options = { - host: '1.1.1.1', + device: { host: '1.1.1.1' }, remotePort: 8080 }; - let stub = sinon.stub(util as any, 'httpGet').resolves({ + let stub = sinon.stub(rokuDeploy, 'sendEcpRequest').resolves({ + status: 200, body: '', - statusCode: 200 + json: undefined }); await rokuECP['doRequest']('query/my-route', options); - expect(stub.getCall(0).args).to.eql([`http://1.1.1.1:8080/query/my-route`, undefined]); - }); - - it('correctly builds url with leading /', async () => { - let options = { - host: '1.1.1.1', - remotePort: 8080 - }; - - let stub = sinon.stub(util as any, 'httpGet').resolves({ - body: '', - statusCode: 200 - }); - - await rokuECP['doRequest']('/query/my-route', options); - expect(stub.getCall(0).args).to.eql([`http://1.1.1.1:8080/query/my-route`, undefined]); + expect(stub.getCall(0).args).to.eql([ + { host: '1.1.1.1' }, + 'query/my-route', + { method: 'GET', ecpPort: 8080, timeout: undefined } + ]); + expect(stub.getCall(0).args[0]).to.equal(options.device); }); - it('passes request options if populated', async () => { + it('maps post to POST and passes the requestOptions timeout', async () => { let options = { - host: '1.1.1.1', - remotePort: 8080, + device: { instanceUrl: 'https://device.rce.roku.com/instance/my-instance', rceToken: 'my-rce-token' }, requestOptions: { timeout: 1000 } }; - let stub = sinon.stub(util as any, 'httpGet').resolves({ + let stub = sinon.stub(rokuDeploy, 'sendEcpRequest').resolves({ + status: 200, body: '', - statusCode: 200 + json: undefined }); - await rokuECP['doRequest']('/query/my-route', options); - expect(stub.getCall(0).args).to.eql([`http://1.1.1.1:8080/query/my-route`, options.requestOptions]); + await rokuECP['doRequest']('/query/my-route', options, 'post'); + expect(stub.getCall(0).args).to.eql([ + options.device, + '/query/my-route', + { method: 'POST', ecpPort: undefined, timeout: 1000 } + ]); }); + }); - it('uses default port 8060 when missing in options', async () => { + describe('getRegistry', () => { + const registryResult: RokuRegistry = { + devId: '12345', + plugins: ['dev'], + spaceAvailable: '32590', + sections: {} + }; + + it('calls rokuDeploy.queryRegistry with the device option', async () => { let options = { - host: '1.1.1.1' + device: { host: '1.1.1.1' }, + remotePort: 8080, + appId: 'dev' }; - let stub = sinon.stub(util as any, 'httpGet').resolves({ - body: '', - statusCode: 200 - }); - - await rokuECP['doRequest']('/query/my-route', options); - expect(stub.getCall(0).args).to.eql([`http://1.1.1.1:8060/query/my-route`, undefined]); + let stub = sinon.stub(rokuDeploy, 'queryRegistry').resolves(registryResult); + + let result = await rokuECP.getRegistry(options); + expect(stub.getCall(0).args).to.eql([{ + device: { host: '1.1.1.1' }, + appId: 'dev', + ecpPort: 8080 + }]); + expect(result).to.eql({ + devId: '12345', + plugins: ['dev'], + sections: {}, + spaceAvailable: '32590', + status: EcpStatus.ok + } as EcpRegistryData); }); - it('supports get and post methods', async () => { + it('passes a local device config through to roku-deploy', async () => { let options = { - host: '1.1.1.1' + remotePort: 8080, + device: { host: '1.1.1.1' }, + appId: 'dev' }; - const getStub = sinon.stub(util as any, 'httpGet').resolves({ - body: '', - statusCode: 200 - }); - const postStub = sinon.stub(util as any, 'httpPost').resolves({ - body: '', - statusCode: 200 - }); - - await rokuECP['doRequest']('/query/my-route', options, 'get'); - expect(getStub.getCall(0).args).to.eql([`http://1.1.1.1:8060/query/my-route`, undefined]); - expect(getStub.callCount).to.eql(1); - expect(postStub.callCount).to.eql(0); - - await rokuECP['doRequest']('/query/my-route', options, 'post'); - expect(postStub.getCall(0).args).to.eql([`http://1.1.1.1:8060/query/my-route`, undefined]); - expect(getStub.callCount).to.eql(1); - expect(postStub.callCount).to.eql(1); + let stub = sinon.stub(rokuDeploy, 'queryRegistry').resolves(registryResult); + await rokuECP.getRegistry(options); + expect(stub.getCall(0).args).to.eql([{ + device: options.device, + appId: 'dev', + ecpPort: 8080 + }]); + expect(stub.getCall(0).args[0].device).to.equal(options.device); }); - }); - describe('getRegistry', () => { - it('calls doRequest with correct route and options', async () => { + it('passes an RCE device config through to roku-deploy', async () => { let options = { - host: '1.1.1.1', - remotePort: 8080, + device: { instanceUrl: 'https://device.rce.roku.com/instance/my-instance', rceToken: 'my-rce-token' }, appId: 'dev' }; - let stub = sinon.stub(rokuECP as any, 'doRequest').resolves({ - body: '', - statusCode: 200 - }); - sinon.stub(rokuECP as any, 'processRegistry').resolves({}); + let stub = sinon.stub(rokuDeploy, 'queryRegistry').resolves(registryResult); await rokuECP.getRegistry(options); - expect(stub.getCall(0).args).to.eql(['query/registry/dev', options]); + expect(stub.getCall(0).args).to.eql([{ + device: options.device, + appId: 'dev', + ecpPort: undefined + }]); + expect(stub.getCall(0).args[0].device).to.equal(options.device); }); - }); - - describe('parseRegistry', () => { - describe('non-error responses', () => { - it('handles ok response with no other properties', async () => { - let response = { - body: ` - - OK - Plugin dev not found - - `, - statusCode: 200 - }; - let result = await rokuECP['processRegistry'](response as any); - expect(result).to.eql({ - devId: undefined, - plugins: undefined, - spaceAvailable: undefined, - sections: {}, - status: EcpStatus.ok - } as EcpRegistryData); - }); - - it('handles ok response with empty sections', async () => { - let response = { - body: ` - - - - 12345 - 12,34,dev - 28075 - - - OK - - `, - statusCode: 200 - }; - - let result = await rokuECP['processRegistry'](response as any); - expect(result).to.eql({ - devId: '12345', - plugins: ['12', '34', 'dev'], - spaceAvailable: '28075', - sections: {}, - status: EcpStatus.ok - } as EcpRegistryData); + it('maps a populated roku-deploy result onto EcpRegistryData', async () => { + sinon.stub(rokuDeploy, 'queryRegistry').resolves({ + devId: '12345', + plugins: ['12', '34', 'dev'], + spaceAvailable: '28075', + sections: { + 'section One': { + 'first key in section one': 'value one section one' + }, + 'section Two': { + 'first key in section two': 'value one section two', + 'second key in section two': 'value two section two' + } + } }); - it('handles ok response with populated sections', async () => { - let response = { - body: ` - - - - 12345 - dev - 32590 - -
- section One - - - first key in section one - value one section one - - -
-
- section Two - - - first key in section two - value one section two - - - second key in section two - value two section two - - -
- - - OK - - ` - }; - let result = await rokuECP['processRegistry'](response as any); - expect(result).to.eql({ - devId: '12345', - plugins: ['dev'], - sections: { - 'section One': { - 'first key in section one': 'value one section one' - }, - 'section Two': { - 'first key in section two': 'value one section two', - 'second key in section two': 'value two section two' - } + let result = await rokuECP.getRegistry({ device: { host: '1.1.1.1' }, appId: 'dev' }); + expect(result).to.eql({ + devId: '12345', + plugins: ['12', '34', 'dev'], + spaceAvailable: '28075', + sections: { + 'section One': { + 'first key in section one': 'value one section one' }, - spaceAvailable: '32590', - status: EcpStatus.ok - } as EcpRegistryData); - }); + 'section Two': { + 'first key in section two': 'value one section two', + 'second key in section two': 'value two section two' + } + }, + status: EcpStatus.ok + } as EcpRegistryData); }); - describe('error responses', () => { - it('handles not in dev mode', async () => { - let response = { - body: ` - - FAILED - Plugin dev not found - - `, - statusCode: 200 - }; - await expectThrowsAsync(() => rokuECP['processRegistry'](response as any), 'Plugin dev not found'); + it('maps a minimal roku-deploy result onto EcpRegistryData', async () => { + sinon.stub(rokuDeploy, 'queryRegistry').resolves({ + sections: {} }); - it('handles device not keyed', async () => { - let response = { - body: ` - - FAILED - Device not keyed - - `, - statusCode: 200 - }; - await expectThrowsAsync(() => rokuECP['processRegistry'](response as any), 'Device not keyed'); - }); + let result = await rokuECP.getRegistry({ device: { host: '1.1.1.1' }, appId: 'dev' }); + expect(result).to.eql({ + devId: undefined, + plugins: undefined, + spaceAvailable: undefined, + sections: {}, + status: EcpStatus.ok + } as EcpRegistryData); + }); - it('handles failed status with missing error', async () => { - let response = { - body: ` - - FAILED - - `, - statusCode: 200 - }; - await expectThrowsAsync(() => rokuECP['processRegistry'](response as any), 'Unknown error'); - }); + it('propagates errors from roku-deploy', async () => { + sinon.stub(rokuDeploy, 'queryRegistry').rejects(new Error('Could not retrieve registry: Device not keyed')); - it('handles error response without xml', async () => { - let response = { - body: `ECP command not allowed in Limited mode.`, - statusCode: 403 - }; - await expectThrowsAsync(() => rokuECP['processRegistry'](response as any), 'ECP command not allowed in Limited mode.'); - }); + await expectThrowsAsync(() => rokuECP.getRegistry({ device: { host: '1.1.1.1' }, appId: 'dev' }), 'Could not retrieve registry: Device not keyed'); }); }); describe('getAppState', () => { - it('calls doRequest with correct route and options', async () => { + const appStateResult: RokuAppState = { + appId: 'dev', + appDevId: '12345', + appTitle: 'my app', + appVersion: '10.0.0', + state: 'active' + }; + + it('calls rokuDeploy.queryAppState with the device option', async () => { let options = { - host: '1.1.1.1', + device: { host: '1.1.1.1' }, remotePort: 8080, appId: 'dev' }; - let stub = sinon.stub(rokuECP as any, 'doRequest').resolves({ - body: '', - statusCode: 200 - }); + let stub = sinon.stub(rokuDeploy, 'queryAppState').resolves(appStateResult); + + let result = await rokuECP.getAppState(options); + expect(stub.getCall(0).args).to.eql([{ + device: { host: '1.1.1.1' }, + appId: 'dev', + ecpPort: 8080 + }]); + expect(result).to.eql({ + appId: 'dev', + appDevId: '12345', + appTitle: 'my app', + appVersion: '10.0.0', + state: AppState.active, + status: EcpStatus.ok + } as EcpAppStateData); + }); + + it('passes an RCE device config through to roku-deploy', async () => { + let options = { + device: { instanceUrl: 'https://device.rce.roku.com/instance/my-instance', rceToken: 'my-rce-token' }, + appId: 'dev' + }; - sinon.stub(rokuECP as any, 'processAppState').resolves({}); + let stub = sinon.stub(rokuDeploy, 'queryAppState').resolves(appStateResult); await rokuECP.getAppState(options); - expect(stub.getCall(0).args).to.eql(['query/app-state/dev', options]); + expect(stub.getCall(0).args).to.eql([{ + device: options.device, + appId: 'dev', + ecpPort: undefined + }]); + expect(stub.getCall(0).args[0].device).to.equal(options.device); }); - }); - - describe('processAppState', () => { - describe('non-error responses', () => { - it('handles ok response', async () => { - let response = { - body: ` - - - dev - my app - 10.0.0 - 12345 - active - OK - - `, - statusCode: 200 - }; - let result = await rokuECP['processAppState'](response as any); - expect(result).to.eql({ - appId: 'dev', - appDevId: '12345', - appTitle: 'my app', - appVersion: '10.0.0', - state: AppState.active, - status: EcpStatus.ok - } as EcpAppStateData); - }); - it('handles ok response with unknown state', async () => { - let response = { - body: ` - - - dev - my app - 10.0.0 - 12345 - bad - OK - - `, - statusCode: 200 - }; - let result = await rokuECP['processAppState'](response as any); - expect(result).to.eql({ - appId: 'dev', - appDevId: '12345', - appTitle: 'my app', - appVersion: '10.0.0', - state: AppState.unknown, - status: EcpStatus.ok - } as EcpAppStateData); + it('maps an unknown state onto AppState.unknown', async () => { + sinon.stub(rokuDeploy, 'queryAppState').resolves({ + ...appStateResult, + state: 'unknown' }); - it('handles ok response with uppercase state', async () => { - let response = { - body: ` - - - dev - my app - 10.0.0 - 12345 - ACTIVE - OK - - `, - statusCode: 200 - }; - let result = await rokuECP['processAppState'](response as any); - expect(result).to.eql({ - appId: 'dev', - appDevId: '12345', - appTitle: 'my app', - appVersion: '10.0.0', - state: AppState.active, - status: EcpStatus.ok - } as EcpAppStateData); - }); + let result = await rokuECP.getAppState({ device: { host: '1.1.1.1' }, appId: 'dev' }); + expect(result).to.eql({ + appId: 'dev', + appDevId: '12345', + appTitle: 'my app', + appVersion: '10.0.0', + state: AppState.unknown, + status: EcpStatus.ok + } as EcpAppStateData); }); - describe('error responses', () => { - it('handles failed status with missing error', async () => { - let response = { - body: ` - - FAILED - - `, - statusCode: 200 - }; - await expectThrowsAsync(() => rokuECP['processAppState'](response as any), 'Unknown error'); - }); + it('propagates errors from roku-deploy', async () => { + sinon.stub(rokuDeploy, 'queryAppState').rejects(new Error('Could not retrieve app state: App not found')); - it('handles failed status with populated error', async () => { - let response = { - body: ` - - FAILED - App not found - - `, - statusCode: 200 - }; - await expectThrowsAsync(() => rokuECP['processAppState'](response as any), 'App not found'); - }); - - it('handles error response without xml', async () => { - let response = { - body: `ECP command not allowed in Limited mode.`, - statusCode: 403 - }; - await expectThrowsAsync(() => rokuECP['processAppState'](response as any), 'ECP command not allowed in Limited mode.'); - }); + await expectThrowsAsync(() => rokuECP.getAppState({ device: { host: '1.1.1.1' }, appId: 'dev' }), 'Could not retrieve app state: App not found'); }); }); describe('exitApp', () => { - it('calls doRequest with correct route and options', async () => { + it('calls rokuDeploy.exitApp with the device option', async () => { let options = { - host: '1.1.1.1', + device: { host: '1.1.1.1' }, remotePort: 8080, appId: 'dev' }; - let stub = sinon.stub(rokuECP as any, 'doRequest').resolves({ - body: '', - statusCode: 200 + let stub = sinon.stub(rokuDeploy, 'exitApp').resolves(); + + let result = await rokuECP.exitApp(options); + expect(stub.getCall(0).args).to.eql([{ + device: { host: '1.1.1.1' }, + appId: 'dev', + ecpPort: 8080 + }]); + expect(result).to.eql({ + status: EcpStatus.ok + }); + }); + + it('passes an RCE device config through to roku-deploy', async () => { + let options = { + device: { instanceUrl: 'https://device.rce.roku.com/instance/my-instance', rceToken: 'my-rce-token' }, + appId: 'dev' + }; + + let stub = sinon.stub(rokuDeploy, 'exitApp').resolves(); + + let result = await rokuECP.exitApp(options); + expect(stub.getCall(0).args).to.eql([{ + device: options.device, + appId: 'dev', + ecpPort: undefined + }]); + expect(stub.getCall(0).args[0].device).to.equal(options.device); + expect(result).to.eql({ + status: EcpStatus.ok }); - sinon.stub(rokuECP as any, 'processExitApp').resolves({}); + }); + + it('propagates errors from roku-deploy', async () => { + sinon.stub(rokuDeploy, 'exitApp').rejects(new Error('Could not exit app: App not found')); - await rokuECP.exitApp(options); - expect(stub.getCall(0).args).to.eql(['exit-app/dev', options, 'post']); + await expectThrowsAsync(() => rokuECP.exitApp({ device: { host: '1.1.1.1' }, appId: 'dev' }), 'Could not exit app: App not found'); }); }); describe('captureHeapSnapshot', () => { it('calls doRequest with correct route and options', async () => { let options = { - host: '1.1.1.1', + device: { host: '1.1.1.1' }, remotePort: 8080, channelId: 'dev' }; @@ -474,7 +345,7 @@ describe('RokuECP', () => { `, statusCode: 200 }); - let result = await rokuECP.captureHeapSnapshot({ host: '1.1.1.1', channelId: 'dev' }); + let result = await rokuECP.captureHeapSnapshot({ device: { host: '1.1.1.1' }, channelId: 'dev' }); expect(result).to.eql({ timestamp: 1772434731151, timestampEnd: 1772434731188, @@ -495,7 +366,7 @@ describe('RokuECP', () => { `, statusCode: 200 }); - await expectThrowsAsync(() => rokuECP.captureHeapSnapshot({ host: '1.1.1.1', channelId: 'dev' }), `Channel 'dev' not running, cannot fetch heap graph`); + await expectThrowsAsync(() => rokuECP.captureHeapSnapshot({ device: { host: '1.1.1.1' }, channelId: 'dev' }), `Channel 'dev' not running, cannot fetch heap graph`); }); it('handles failed status with missing error', async () => { @@ -508,7 +379,7 @@ describe('RokuECP', () => { `, statusCode: 200 }); - await expectThrowsAsync(() => rokuECP.captureHeapSnapshot({ host: '1.1.1.1', channelId: 'dev' }), 'Unknown error'); + await expectThrowsAsync(() => rokuECP.captureHeapSnapshot({ device: { host: '1.1.1.1' }, channelId: 'dev' }), 'Unknown error'); }); it('handles error response without xml', async () => { @@ -516,63 +387,9 @@ describe('RokuECP', () => { body: `ECP command not allowed in Limited mode.`, statusCode: 403 }); - await expectThrowsAsync(() => rokuECP.captureHeapSnapshot({ host: '1.1.1.1', channelId: 'dev' }), 'ECP command not allowed in Limited mode.'); + await expectThrowsAsync(() => rokuECP.captureHeapSnapshot({ device: { host: '1.1.1.1' }, channelId: 'dev' }), 'ECP command not allowed in Limited mode.'); }); }); }); - describe('processExitApp', () => { - describe('non-error responses', () => { - it('handles ok response', async () => { - let response = { - body: ` - - - OK - - `, - statusCode: 200 - }; - let result = await rokuECP['processExitApp'](response as any); - expect(result).to.eql({ - status: EcpStatus.ok - }); - }); - }); - - describe('error responses', () => { - it('handles failed status with missing error', async () => { - let response = { - body: ` - - FAILED - - `, - statusCode: 200 - }; - await expectThrowsAsync(() => rokuECP['processExitApp'](response as any), 'Unknown error'); - }); - - it('handles failed status with populated error', async () => { - let response = { - body: ` - - FAILED - App not found - - `, - statusCode: 200 - }; - await expectThrowsAsync(() => rokuECP['processExitApp'](response as any), 'App not found'); - }); - - it('handles error response without xml', async () => { - let response = { - body: `ECP command not allowed in Limited mode.`, - statusCode: 403 - }; - await expectThrowsAsync(() => rokuECP['processExitApp'](response as any), 'ECP command not allowed in Limited mode.'); - }); - }); - }); }); diff --git a/src/RokuECP.ts b/src/RokuECP.ts index c0a843b2..b465fe89 100644 --- a/src/RokuECP.ts +++ b/src/RokuECP.ts @@ -1,15 +1,19 @@ import { util } from './util'; import type * as requestType from 'request'; -import type { Response } from 'request'; +import { rokuDeploy } from 'roku-deploy'; +import type { DeviceConfig, EcpResult } from 'roku-deploy'; export class RokuECP { - private async doRequest(route: string, options: BaseOptions, method: 'post' | 'get' = 'get'): Promise { - const url = `http://${options.host}:${options.remotePort ?? 8060}/${route.replace(/^\//, '')}`; - if (method === 'post') { - return util.httpPost(url, options.requestOptions); - } else { - return util.httpGet(url, options.requestOptions); - } + /** + * Send a raw ECP request through roku-deploy's `sendEcpRequest()` transport, which routes a local device + * over plain HTTP and a Roku Cloud Emulator device through its instance's ECP proxy. + */ + private async doRequest(route: string, options: BaseOptions, method: 'post' | 'get' = 'get'): Promise { + return rokuDeploy.sendEcpRequest(options.device, route, { + method: method === 'post' ? 'POST' : 'GET', + ecpPort: options.remotePort, + timeout: options.requestOptions?.timeout + }); } /** @@ -20,7 +24,7 @@ export class RokuECP { public async enablePerfettoTracing(options: BaseOptions & { channelId: string }) { const response = await this.doRequest(`/perfetto/enable/${options.channelId}`, options, 'post'); - return this.parseResponse(response, 'perfetto-enable', (parsed: PerfettoEnableAsJson, status): EcpPerfettoEnableData => { + return this.parseResponse(response.body, 'perfetto-enable', (parsed: PerfettoEnableAsJson, status): EcpPerfettoEnableData => { return { enabledChannels: parsed?.['enabled-channels']?.[0]?.channel ?? [], timestamp: Number(parsed?.timestamp?.[0]), @@ -37,7 +41,7 @@ export class RokuECP { */ public async captureHeapSnapshot(options: BaseOptions & { channelId: string }) { const response = await this.doRequest(`/perfetto/heapgraph/trigger/${options.channelId}`, options, 'post'); - return this.parseResponse(response, 'perfetto-heapgraph-trigger', (parsed: HeapSnapshotAsJson, status): EcpHeapSnapshotData => { + return this.parseResponse(response.body, 'perfetto-heapgraph-trigger', (parsed: HeapSnapshotAsJson, status): EcpHeapSnapshotData => { return { timestamp: Number(parsed?.timestamp?.[0]), timestampEnd: Number(parsed?.['timestamp-end']?.[0]), @@ -50,14 +54,14 @@ export class RokuECP { return EcpStatus[response?.[rootKey]?.status?.[0]?.toLowerCase()] ?? EcpStatus.failed; } - private async parseResponse(response: Response, rootKey: string, callback: (parsed: any, status: EcpStatus) => R): Promise { - if (typeof response.body === 'string') { + private async parseResponse(body: string, rootKey: string, callback: (parsed: any, status: EcpStatus) => R): Promise { + if (typeof body === 'string') { let parsed: ParsedEcpRoot; try { - parsed = await util.parseXml(response.body); + parsed = await util.parseXml(body); } catch { //if the response is not xml, just return the body as-is - throw new Error(response.body ?? 'Unknown error'); + throw new Error(body ?? 'Unknown error'); } const status = this.getEcpStatus(parsed, rootKey); @@ -69,65 +73,44 @@ export class RokuECP { } } - public async getRegistry(options: BaseOptions & { appId: string }) { - let result = await this.doRequest(`query/registry/${options.appId}`, options); - return this.processRegistry(result); - } - - private async processRegistry(response: Response) { - return this.parseResponse(response, 'plugin-registry', (parsed: RegistryAsJson, status): EcpRegistryData => { - const registry = parsed?.registry?.[0]; - let sections: EcpRegistryData['sections'] = {}; - - for (const section of registry?.sections?.[0]?.section ?? []) { - if (typeof section === 'string') { - continue; - } - let sectionName = section.name[0]; - for (const item of section.items[0].item) { - sections[sectionName] ??= {}; - sections[sectionName][item.key[0]] = item.value[0]; - } - } - - return { - devId: registry?.['dev-id']?.[0], - plugins: registry?.plugins?.[0]?.split(','), - sections: sections, - spaceAvailable: registry?.['space-available']?.[0], - status: status - }; + public async getRegistry(options: BaseOptions & { appId: string }): Promise { + const registry = await rokuDeploy.queryRegistry({ + device: options.device, + appId: options.appId, + ecpPort: options.remotePort }); + return { + devId: registry.devId, + plugins: registry.plugins, + sections: registry.sections, + spaceAvailable: registry.spaceAvailable, + status: EcpStatus.ok + }; } - public async getAppState(options: BaseOptions & { appId: string }) { - let result = await this.doRequest(`query/app-state/${options.appId}`, options); - return this.processAppState(result); - } - - private async processAppState(response: Response) { - return this.parseResponse(response, 'app-state', (parsed: AppStateAsJson, status): EcpAppStateData => { - const state = AppState[parsed.state?.[0]?.toLowerCase()] ?? AppState.unknown; - return { - appId: parsed['app-id']?.[0], - appDevId: parsed['app-dev-id']?.[0], - appTitle: parsed['app-title']?.[0], - appVersion: parsed['app-version']?.[0], - state: state, - status: status - }; + public async getAppState(options: BaseOptions & { appId: string }): Promise { + const appState = await rokuDeploy.queryAppState({ + device: options.device, + appId: options.appId, + ecpPort: options.remotePort }); + return { + appId: appState.appId, + appDevId: appState.appDevId, + appTitle: appState.appTitle, + appVersion: appState.appVersion, + state: AppState[appState.state] ?? AppState.unknown, + status: EcpStatus.ok + }; } public async exitApp(options: BaseOptions & { appId: string }): Promise { - let result = await this.doRequest(`exit-app/${options.appId}`, options, 'post'); - return this.processExitApp(result); - } - - private async processExitApp(response: Response): Promise { - return this.parseResponse(response, 'exit-app', (parsed: ExitAppAsJson, status): EcpExitAppData => { - return { status: status }; + await rokuDeploy.exitApp({ + device: options.device, + appId: options.appId, + ecpPort: options.remotePort }); + return { status: EcpStatus.ok }; } } @@ -136,9 +119,14 @@ export enum EcpStatus { failed = 'failed' } interface BaseOptions { - host: string; remotePort?: number; requestOptions?: requestType.CoreOptions; + /** + * The roku-deploy device config for the target device. When this is an RCE device config, + * roku-deploy routes the request through the instance's ECP proxy instead of the local HTTP + * ECP endpoint. + */ + device: DeviceConfig; } interface BaseEcpResponse { @@ -157,25 +145,6 @@ interface ParsedEcpBase { error?: [string]; } -interface RegistryAsJson extends ParsedEcpBase { - registry: [{ - 'dev-id': [string]; - plugins: [string]; - sections: [{ - section: [{ - items: [{ - item: [{ - key: [string]; - value: [string]; - }]; - }]; - name: [string]; - } | string]; - }]; - 'space-available': [string]; - }]; -} - export interface EcpRegistryData extends BaseEcpResponse { devId?: string; plugins?: Array; @@ -183,14 +152,6 @@ export interface EcpRegistryData extends BaseEcpResponse { spaceAvailable?: string; state?: string; } -interface AppStateAsJson extends ParsedEcpBase { - 'app-id': [string]; - 'app-title': [string]; - 'app-version': [string]; - 'app-dev-id': [string]; - state: ['active' | 'background' | 'inactive']; -} - export enum AppState { active = 'active', background = 'background', @@ -220,8 +181,6 @@ export interface EcpPerfettoEnableData extends BaseEcpResponse { timestampEnd?: number; } -type ExitAppAsJson = ParsedEcpBase; - export interface EcpExitAppData { status: EcpStatus; errorMessage?: string; diff --git a/src/SceneGraphDebugCommandController.spec.ts b/src/SceneGraphDebugCommandController.spec.ts index 73aef71e..e424a072 100644 --- a/src/SceneGraphDebugCommandController.spec.ts +++ b/src/SceneGraphDebugCommandController.spec.ts @@ -1,5 +1,6 @@ import * as sinon from 'sinon'; import { expect } from 'chai'; +import * as stream from 'stream'; import { SceneGraphDebugCommandController } from './SceneGraphDebugCommandController'; describe('SceneGraphDebugCommandController ', () => { @@ -7,7 +8,7 @@ describe('SceneGraphDebugCommandController ', () => { let execStub: sinon.SinonStub; beforeEach(() => { - commandController = new SceneGraphDebugCommandController('192.168.1.1'); + commandController = new SceneGraphDebugCommandController({ host: '192.168.1.1' }); commandController['connection'] = {}; execStub = sinon.stub(commandController, 'exec').callsFake((command: string) => { return new Promise((resolve) => { @@ -232,3 +233,183 @@ describe('SceneGraphDebugCommandController ', () => { }); }); }); + +/** + * Minimal fake standing in for roku-deploy's RokuDeploySocket. Genuinely extends `stream.Duplex` + * (rather than just an EventEmitter) because the controller hands this straight to telnet-client as + * an injected `sock`, and telnet-client's `_checkSocket()` guard requires `pipe`, `_write`, + * `_writableState`, `_read`, and `_readableState`, all of which only a real Node stream provides. + */ +class FakeRokuDeploySocket extends stream.Duplex { + public writtenChunks: string[] = []; + + /** + * Text pushed shortly after connect(), simulating the device's connection greeting: a leading + * blank line followed by a bare `>` prompt with no trailing newline. Both device types deliver + * this shape (the Roku Cloud Emulator's instance api ports routes pass the un-terminated `>` + * through byte-unchanged); the controller has to consume it itself because telnet-client skips + * its own prompt wait for an injected sock. + */ + public initialGreeting = '\r\n>'; + + /** + * When set, connect() emits 'error' instead of 'connect', simulating a transport-level connect + * failure (the tcp handshake or websocket handshake itself failing). + */ + public connectShouldFail: Error | undefined; + + /** + * Canned response text to push, one per queued entry, the next time data is written to this + * socket. Each response is pushed on a later tick so it always arrives after the write that + * triggered it, the same way a real device's response arrives strictly after the command that + * produced it. + */ + public queuedResponses: string[] = []; + + public connect(connectListener?: () => void): this { + if (this.connectShouldFail) { + setTimeout(() => { + this.emit('error', this.connectShouldFail); + }, 0); + return this; + } + if (connectListener) { + this.once('connect', connectListener); + } + setTimeout(() => { + this.emit('connect'); + }, 0); + setTimeout(() => { + this.push(Buffer.from(this.initialGreeting)); + }, 10); + return this; + } + + public _write(chunk: Buffer, encoding: BufferEncoding, callback: (error?: Error | null) => void): void { + this.writtenChunks.push(chunk.toString()); + let response = this.queuedResponses.shift(); + if (response !== undefined) { + setTimeout(() => { + this.push(Buffer.from(response)); + }, 0); + } + callback(); + } + + public _read(size: number): void { + //data is pushed as it is scheduled above; nothing to pull on demand + } + + /** + * telnet-client calls this unconditionally right after adopting an injected sock (regardless of + * whether a timeout was ever configured), so it has to exist even though telnet-client's + * `_checkSocket()` guard itself never checks for it. A missing implementation would throw + * synchronously inside telnet-client's connect() executor, silently skipping every listener it + * registers after that point ('data' included) even though the promise had already resolved. + */ + public setTimeout(milliseconds: number, callback?: () => void): this { + return this; + } +} + +describe('SceneGraphDebugCommandController transport', () => { + let controller: SceneGraphDebugCommandController; + let fakeRokuDeploySocket: FakeRokuDeploySocket; + let createRokuDeploySocketStub: sinon.SinonStub; + + beforeEach(() => { + controller = new SceneGraphDebugCommandController({ host: '192.168.1.50' }); + fakeRokuDeploySocket = new FakeRokuDeploySocket(); + createRokuDeploySocketStub = sinon.stub(controller as any, 'createRokuDeploySocket').returns(fakeRokuDeploySocket); + }); + + afterEach(() => { + sinon.restore(); + }); + + describe('createRokuDeploySocket factory', () => { + it('passes the local device config and the configured port through', async () => { + await controller.connect(); + + expect(createRokuDeploySocketStub.calledOnce).to.be.true; + let options = createRokuDeploySocketStub.firstCall.args[0]; + expect(options.device).to.eql({ host: '192.168.1.50' }); + expect(options.port).to.equal(8080); + }); + + it('passes an RCE device config through verbatim when constructed with one', async () => { + let rceDevice = { instanceUrl: 'https://device.rce.roku.com/instance/abc', rceToken: 'token-value' }; + let rceController = new SceneGraphDebugCommandController(rceDevice, 8080); + let rceFakeRokuDeploySocket = new FakeRokuDeploySocket(); + let rceCreateRokuDeploySocketStub = sinon.stub(rceController as any, 'createRokuDeploySocket').returns(rceFakeRokuDeploySocket); + + await rceController.connect(); + + expect(rceCreateRokuDeploySocketStub.calledOnce).to.be.true; + let options = rceCreateRokuDeploySocketStub.firstCall.args[0]; + expect(options.device).to.equal(rceDevice); + expect(options.port).to.equal(8080); + }); + }); + + describe('connect', () => { + it('consumes the connect greeting before handing the socket to telnet-client, so it does not pollute the first exec response', async () => { + await controller.connect(); + + expect(controller['connection']).to.exist; + expect(createRokuDeploySocketStub.calledOnce).to.be.true; + + fakeRokuDeploySocket.queuedResponses.push('abc123\r\n>'); + let response = await controller.exec('showkey'); + + //if the greeting had not been consumed first, its bytes would still be sitting in front + //of the real response text here + expect(response.error).to.be.undefined; + expect(response.result.rawResponse).to.include('abc123'); + expect(response.result.rawResponse.startsWith('>')).to.be.false; + }); + + it('rejects and destroys the socket when the shell prompt never arrives', async () => { + //a greeting with no prompt in it: the prompt wait can never complete + fakeRokuDeploySocket.initialGreeting = 'some banner text\r\n'; + + let thrownError: Error | undefined; + try { + await controller.connect({ timeout: 50 }); + } catch (e) { + thrownError = e as Error; + } + + expect(thrownError).to.be.instanceOf(Error); + expect(thrownError.message).to.include(`waiting for the SceneGraph debug server's shell prompt`); + expect(fakeRokuDeploySocket.destroyed).to.be.true; + expect(controller['connection']).to.be.null; + }); + + it('does not crash when the socket errors after the connection is established', async () => { + await controller.connect(); + + //a transport error on the live connection (a device reboot mid-session, for example) + //must be swallowed by the controller's own listener rather than crashing the process + expect(() => { + fakeRokuDeploySocket.emit('error', new Error('read ECONNRESET')); + }).not.to.throw(); + }); + + it('destroys the socket when the transport connect fails', async () => { + fakeRokuDeploySocket.connectShouldFail = new Error('connect ECONNREFUSED 192.168.1.50:8080'); + + let thrownError: Error | undefined; + try { + await controller.connect(); + } catch (e) { + thrownError = e as Error; + } + + expect(thrownError).to.be.instanceOf(Error); + expect(thrownError.message).to.include('ECONNREFUSED'); + expect(fakeRokuDeploySocket.destroyed).to.be.true; + expect(controller['connection']).to.be.null; + }); + }); +}); diff --git a/src/SceneGraphDebugCommandController.ts b/src/SceneGraphDebugCommandController.ts index b8dc90d6..11485dc6 100644 --- a/src/SceneGraphDebugCommandController.ts +++ b/src/SceneGraphDebugCommandController.ts @@ -1,12 +1,20 @@ +import { createRokuDeploySocket } from 'roku-deploy'; +import type { DeviceConfig, RokuDeploySocket, SocketOptions } from 'roku-deploy'; import { logger } from './logging'; // eslint-disable-next-line const Telnet = require('telnet-client'); export class SceneGraphDebugCommandController { - constructor(public host: string, port?: number) { + constructor(device: DeviceConfig, port?: number) { + this.device = device; this.port = port ?? 8080; } + /** + * The roku-deploy device config for the target device + */ + private device: DeviceConfig; + private connection: typeof Telnet; private shellPrompt = /^>$/img; @@ -17,10 +25,38 @@ export class SceneGraphDebugCommandController { private maxBufferLength = 5242880; private logger = logger.createLogger(`[${SceneGraphDebugCommandController.name}]`); + /** + * Create the transport used to reach the SceneGraph debug server. Extracted to a protected + * method so tests can substitute a fake socket. + */ + protected createRokuDeploySocket(options: SocketOptions): RokuDeploySocket { + return createRokuDeploySocket(options); + } + public async connect(options: { execTimeout?: number; timeout?: number } = {}) { this.removeConnection(); + const timeoutMs = options.timeout ?? this.timeout; + + let socket: RokuDeploySocket | undefined; try { + socket = this.createRokuDeploySocket({ + device: this.device, + port: this.port + }); + //keep an error listener attached for the socket's whole life: waitForConnectPrompt and + //telnet-client each remove or narrow theirs at various points, and a socket 'error' + //emitted while no listener is attached would crash the whole process + socket.on('error', (error: Error) => { + this.logger.debug('SceneGraph debug server socket error', error); + }); + + await this.waitForSocketConnect(socket, timeoutMs); + //an injected sock skips telnet-client's own prompt wait entirely (see the comment on + //waitForConnectPrompt below), so the greeting has to be consumed here first or it would + //otherwise arrive mid-exec and prematurely terminate the first command's response + await this.waitForConnectPrompt(socket, timeoutMs); + // Make a new telnet connections object let connection = new Telnet(); @@ -28,23 +64,97 @@ export class SceneGraphDebugCommandController { this.removeConnection(); }); const config = { - host: this.host, port: this.port, shellPrompt: this.shellPrompt, echoLines: this.echoLines, timeout: this.timeout, execTimeout: this.execTimeout, maxBufferLength: this.maxBufferLength, - ...options + ...options, + sock: socket }; this.logger.debug('Establishing telnet connection', config); await connection.connect(config); this.connection = connection; } catch (e) { + //the socket was created but never became the live connection, so nothing else will ever + //destroy it. Leaving it dangling would leak an open socket or websocket. + socket?.destroy(); throw new Error((e as Error).message); } } + /** + * Waits for the transport-level connection to establish (the tcp handshake for a local device, + * or the websocket handshake for an RCE device), racing its 'connect' event against 'error' and + * a manual timeout. An injected sock bypasses telnet-client's own connect timeout entirely + * (telnet-client resolves immediately for an injected sock, before its timeout is even armed), + * so this is the only thing enforcing one here. + */ + private waitForSocketConnect(socket: RokuDeploySocket, timeoutMs: number): Promise { + return new Promise((resolve, reject) => { + const cleanup = () => { + clearTimeout(timeoutHandle); + socket.removeListener('connect', onConnect); + socket.removeListener('error', onError); + }; + const onConnect = () => { + cleanup(); + resolve(); + }; + const onError = (error: Error) => { + cleanup(); + reject(error); + }; + const timeoutHandle = setTimeout(() => { + cleanup(); + reject(new Error(`Timed out connecting to the SceneGraph debug server after ${timeoutMs}ms`)); + }, timeoutMs); + + socket.once('connect', onConnect); + socket.once('error', onError); + socket.connect(); + }); + } + + /** + * Waits for the connection greeting (a bare `>` shell prompt) to arrive and consumes it before + * the socket is handed to telnet-client. This mirrors the prompt wait telnet-client performs + * when it owns the socket, which it skips entirely for an injected sock (it treats one as + * already `'ready'`); without it the greeting would instead arrive mid-exec, where its prompt + * would prematurely terminate the first command's response. Rejects if the prompt does not + * arrive within `timeoutMs`. + */ + private waitForConnectPrompt(socket: RokuDeploySocket, timeoutMs: number): Promise { + return new Promise((resolve, reject) => { + let accumulatedText = ''; + //a fresh non-global copy avoids a stateful lastIndex across calls, since + //this.shellPrompt carries the 'g' flag + const nonGlobalShellPrompt = new RegExp(this.shellPrompt.source, this.shellPrompt.flags.replace('g', '')); + + const finish = (error?: Error) => { + socket.removeListener('data', onData); + clearTimeout(timeoutHandle); + if (error) { + reject(error); + } else { + resolve(); + } + }; + const onData = (chunk: Buffer) => { + accumulatedText += chunk.toString('utf8'); + if (nonGlobalShellPrompt.test(accumulatedText)) { + finish(); + } + }; + const timeoutHandle = setTimeout(() => { + finish(new Error(`Timed out after ${timeoutMs}ms waiting for the SceneGraph debug server's shell prompt`)); + }, timeoutMs); + + socket.on('data', onData); + }); + } + private removeConnection() { this.connection = null; } diff --git a/src/adapters/DebugProtocolAdapter.spec.ts b/src/adapters/DebugProtocolAdapter.spec.ts index 2bd8b585..00c5f377 100644 --- a/src/adapters/DebugProtocolAdapter.spec.ts +++ b/src/adapters/DebugProtocolAdapter.spec.ts @@ -1,7 +1,7 @@ /* eslint-disable prefer-arrow-callback */ import { expect } from 'chai'; -import type { DebugProtocolClient } from '../debugProtocol/client/DebugProtocolClient'; +import { DebugProtocolClient } from '../debugProtocol/client/DebugProtocolClient'; import { ProtocolCapabilities } from '../debugProtocol/client/ProtocolCapabilities'; import { DebugProtocolAdapter, KeyType } from './DebugProtocolAdapter'; import { createSandbox } from 'sinon'; @@ -28,9 +28,49 @@ import { RemoveBreakpointsRequest } from '../debugProtocol/events/requests/Remov import type { AfterSendRequestEvent } from '../debugProtocol/client/DebugProtocolClientPlugin'; import { GenericV3Response } from '../debugProtocol/events/responses/GenericV3Response'; import { RendezvousTracker } from '../RendezvousTracker'; -import { Socket } from 'net'; +import { EventEmitter } from 'events'; const sinon = createSandbox(); +/** + * Minimal fake standing in for roku-deploy's RokuDeploySocket. A real RokuDeploySocket is itself an + * EventEmitter ('connect'|'ready'|'data'|'close'|'error', ...), so extending Node's EventEmitter + * directly gives correct on/removeListener/emit semantics. + */ +class FakeRokuDeploySocket extends EventEmitter { + public writtenChunks: Array = []; + + public destroyed = false; + + /** + * Mirrors RokuDeploySocket#connect(): emits 'connect' then 'ready', then invokes the connect + * listener, exactly like net.Socket does. + */ + public connect(connectListener?: () => void): this { + this.emit('connect'); + this.emit('ready'); + connectListener?.(); + return this; + } + + public write(data: string | Buffer): boolean { + this.writtenChunks.push(data); + return true; + } + + public destroy(): this { + this.destroyed = true; + return this; + } + + public end(): this { + return this; + } + + public setTimeout(timeout: number, callback?: () => void): this { + return this; + } +} + let cwd = s`${process.cwd()}`; let tmpDir = s`${cwd}/.tmp`; let rootDir = s`${tmpDir}/rootDir`; @@ -60,9 +100,11 @@ describe('DebugProtocolAdapter', function() { beforeEach(async () => { sinon.stub(console, 'log').callsFake((...args) => { }); + //`device` addresses the adapter's client sockets; `host` remains only as the DebugProtocolServer bind address const options = { controlPort: undefined as number, - host: '127.0.0.1' + host: '127.0.0.1', + device: { host: '127.0.0.1' } }; const sourcemapManager = new SourceMapManager(); const locationManager = new LocationManager(sourcemapManager); @@ -152,6 +194,33 @@ describe('DebugProtocolAdapter', function() { await adapter.getStackTrace(0); } + describe('createDebugProtocolClient', () => { + it('bails out gracefully when the client closes while connect is in flight', async () => { + sinon.stub(adapter, 'processTelnetOutput').callsFake(async () => { }); + await adapter.connect(); + //filters that were queued before the client existed + adapter['pendingExceptionBreakpointFilters'] = [{ filter: 'caught' }] as any; + + sinon.stub(DebugProtocolClient.prototype, 'destroy').resolves(); + sinon.stub(DebugProtocolClient.prototype, 'connect').callsFake(async function connect(this: DebugProtocolClient) { + //the device kills the session it just accepted: the client's emit defers a tick, so + //stay "connecting" long enough for the adapter's 'close' handler to run (clearing + //adapter.client) while this connect is still settling + (this as any).emit('close'); + await util.sleep(10); + return true; + }); + + //must not throw (this used to crash reading setExceptionBreakpoints off the cleared client) + await adapter['createDebugProtocolClient'](); + + expect(adapter['client']).to.be.undefined; + expect(adapter['connected']).to.be.false; + //the queued filters survive for the next connection + expect(adapter['pendingExceptionBreakpointFilters']).to.eql([{ filter: 'caught' }]); + }); + }); + describe('getStackTrace', () => { it('recovers when there are no stack frames', async () => { await initialize(); @@ -770,10 +839,9 @@ describe('DebugProtocolAdapter', function() { it('does not crash and triggers shutdown when the socket errors after the connection is established', async () => { // Stub the settle method so processTelnetOutput completes without a real connection sinon.stub(adapter as any, 'settleCompileClient').resolves(''); - // Stub Socket.prototype.connect so it doesn't attempt a real connection - sinon.stub(Socket.prototype, 'connect').callsFake(function(this: Socket) { - return this; - }); + // Inject a fake telnet socket instead of opening a real one + const fakeRokuDeploySocket = new FakeRokuDeploySocket(); + sinon.stub(adapter as any, 'createRokuDeploySocket').returns(fakeRokuDeploySocket); await adapter.processTelnetOutput(); diff --git a/src/adapters/DebugProtocolAdapter.ts b/src/adapters/DebugProtocolAdapter.ts index ab4c2209..9556083b 100644 --- a/src/adapters/DebugProtocolAdapter.ts +++ b/src/adapters/DebugProtocolAdapter.ts @@ -1,5 +1,6 @@ import * as EventEmitter from 'events'; -import { Socket } from 'net'; +import { createRokuDeploySocket } from 'roku-deploy'; +import type { RokuDeploySocket, SocketOptions } from 'roku-deploy'; import { DiagnosticSeverity, util as bscUtil } from 'brighterscript'; import type { BSDebugDiagnostic } from '../CompileErrorProcessor'; import { CompileErrorProcessor } from '../CompileErrorProcessor'; @@ -73,7 +74,7 @@ export class DebugProtocolAdapter { */ public connected: boolean; - private compileClient: Socket; + private compileClient: RokuDeploySocket; private compileErrorProcessor: CompileErrorProcessor; private emitter: EventEmitter; private chanperfTracker: ChanperfTracker; @@ -231,7 +232,7 @@ export class DebugProtocolAdapter { * @param client * @param maxWaitMilliseconds */ - private settleCompileClient(client: Socket, maxWaitMilliseconds = 400) { + private settleCompileClient(client: RokuDeploySocket, maxWaitMilliseconds = 400) { return new Promise((resolve) => { let timeoutStarted = false; let callCount = -1; @@ -280,6 +281,14 @@ export class DebugProtocolAdapter { return this.firstConnectDeferred.promise; } + /** + * Create the transport used to reach the device's BrightScript console. Extracted to a + * protected method so tests can substitute a fake socket. + */ + protected createRokuDeploySocket(options: SocketOptions): RokuDeploySocket { + return createRokuDeploySocket(options); + } + /** * Connect to the telnet session. This should be called before the channel is launched. */ @@ -389,7 +398,7 @@ export class DebugProtocolAdapter { //if there were any unsuccessful breakpoint verifications, we need to ask the device to delete those breakpoints as they've gone missing on our side if (unverifiableDeviceIds.length > 0) { this.logger.warn('Could not find breakpoints to verify. Removing from device:', { deviceBreakpointIds: unverifiableDeviceIds }); - void this.client.removeBreakpoints(unverifiableDeviceIds); + void this.client?.removeBreakpoints(unverifiableDeviceIds); } this.emit('breakpoints-verified', event); }); @@ -408,7 +417,17 @@ export class DebugProtocolAdapter { await this.client.connect(); - this.logger.log(`Connected to device`, { host: this.options.host, connected: this.connected }); + //the client can be torn down while the connect above is still settling (its 'close' + //handler clears `this.client` - for example the device immediately killing the session + //it accepted). Everything below configures a client that no longer exists, so bail and + //leave the queued breakpoint state for the next connection instead of crashing. + if (!this.client) { + this.logger.warn('Debug protocol client closed before setup completed; waiting for a new connection'); + deferred.resolve(); + return await deferred.promise; + } + + this.logger.log(`Connected to device`, { device: util.getDeviceLabel(this.options.device), connected: this.connected }); this.connected = true; this.isAppRunning = true; this.handleStartupIfReady(); @@ -463,7 +482,11 @@ export class DebugProtocolAdapter { let deferred = defer(); try { - this.compileClient = new Socket({ allowHalfOpen: false }); + //normalizeAdapterOptions guarantees `device` is a concrete device config + const device = this.options.device; + const deviceLabel = util.getDeviceLabel(device); + + this.compileClient = this.createRokuDeploySocket({ device: device, port: this.options.brightScriptConsolePort }); util.registerSocketLogging(this.compileClient, this.logger, 'CompileClient'); this.compileErrorProcessor.on('diagnostics', (errors) => { @@ -480,11 +503,11 @@ export class DebugProtocolAdapter { //After a successful connection the deferred is already resolved, so a post-connection //socket error (e.g. ECONNRESET on device disconnect) must not crash the process. this.compileClient.on('error', (err) => { - deferred.tryReject(new Error(`Error with connection to: ${this.options.host}:${this.options.brightScriptConsolePort} \n\n ${err.message} `)); + deferred.tryReject(new Error(`Error with connection to: ${deviceLabel}:${this.options.brightScriptConsolePort} \n\n ${err.message} `)); }); - this.logger.info('Connecting via telnet to gather compile info', { host: this.options.host, port: this.options.brightScriptConsolePort }); - this.compileClient.connect(this.options.brightScriptConsolePort, this.options.host, () => { - this.logger.log(`CONNECTED via telnet to gather compile info`, { host: this.options.host, port: this.options.brightScriptConsolePort }); + this.logger.info('Connecting via telnet to gather compile info', { device: deviceLabel, port: this.options.brightScriptConsolePort }); + this.compileClient.connect(() => { + this.logger.log(`CONNECTED via telnet to gather compile info`, { device: deviceLabel, port: this.options.brightScriptConsolePort }); }); this.logger.debug('Waiting for the compile client to settle'); @@ -493,14 +516,14 @@ export class DebugProtocolAdapter { this.logger.trace('Settled logs:', settledLogs); if (settledLogs.trim().startsWith('Console connection is already in use.')) { - throw new SocketConnectionInUseError(`Telnet connection ${this.options.host}:${this.options.brightScriptConsolePort} already is use`, { + throw new SocketConnectionInUseError(`Telnet connection ${deviceLabel}:${this.options.brightScriptConsolePort} already is use`, { port: this.options.brightScriptConsolePort, - host: this.options.host + host: deviceLabel }); } let lastPartialLine = ''; - this.compileClient.on('data', (buffer) => { + this.compileClient.on('data', (buffer: Buffer) => { let responseText = buffer.toString(); this.logger.info('CompileClient received data', { responseText }); diff --git a/src/adapters/TelnetAdapter.spec.ts b/src/adapters/TelnetAdapter.spec.ts index 1cfaaf82..aed3a9e3 100644 --- a/src/adapters/TelnetAdapter.spec.ts +++ b/src/adapters/TelnetAdapter.spec.ts @@ -1,16 +1,52 @@ import { expect } from 'chai'; import { TelnetAdapter } from './TelnetAdapter'; import * as dedent from 'dedent'; +import { EventEmitter } from 'events'; import { HighLevelType } from '../interfaces'; import { RendezvousTracker } from '../RendezvousTracker'; import type { LaunchConfiguration } from '../LaunchConfiguration'; import type { EvaluateContainer } from './DebugProtocolAdapter'; import { createSandbox } from 'sinon'; -import { Socket } from 'net'; import { rokuDeploy } from 'roku-deploy'; +import type { SocketOptions } from 'roku-deploy'; const sinon = createSandbox(); +/** + * Minimal fake standing in for roku-deploy's RokuDeploySocket. A real RokuDeploySocket is itself an + * EventEmitter ('connect'|'ready'|'data'|'close'|'error', ...), so extending Node's EventEmitter + * directly gives correct on/removeListener/emit semantics. TelnetAdapter never hands this to + * telnet-client (that is only SceneGraphDebugCommandController's concern), so it does not need to + * satisfy telnet-client's `_checkSocket()` stream-internals requirements. + */ +class FakeRokuDeploySocket extends EventEmitter { + public writtenChunks: Array = []; + + public destroyed = false; + + /** + * Mirrors RokuDeploySocket#connect(): emits 'connect' then 'ready', then invokes the connect + * listener, exactly like net.Socket does. + */ + public connect(connectListener?: () => void): this { + this.emit('connect'); + this.emit('ready'); + connectListener?.(); + return this; + } + + public write(data: string | Buffer): boolean { + this.writtenChunks.push(data); + return true; + } + + public destroy(): this { + this.destroyed = true; + this.emit('close'); + return this; + } +} + describe('TelnetAdapter ', () => { let adapter: TelnetAdapter; let launchConfig = { @@ -25,7 +61,7 @@ describe('TelnetAdapter ', () => { beforeEach(() => { adapter = new TelnetAdapter( { - host: '127.0.0.1' + device: { host: '127.0.0.1' } }, rendezvousTracker ); @@ -256,46 +292,80 @@ describe('TelnetAdapter ', () => { }); describe('connect', () => { - it('does not crash and triggers shutdown when the socket errors after the connection is established', async () => { - // Stub pressHomeButton so we don't need a real device - sinon.stub(rokuDeploy, 'pressHomeButton').resolves(); - // Stub Socket.prototype.connect so it doesn't attempt a real connection. - // The callback is invoked synchronously to simulate a successful connection. - sinon.stub(Socket.prototype, 'connect').callsFake(function(this: Socket, ...args: any[]) { - const cb = args.find(a => typeof a === 'function'); - if (cb) { - cb(); - } - return this; - }); + it('does not crash and triggers shutdown when the connection errors after it is established', async () => { + // Stub keyPress so we don't need a real device + sinon.stub(rokuDeploy, 'keyPress').resolves(); + // Inject a fake telnet socket instead of opening a real one + const fakeRokuDeploySocket = new FakeRokuDeploySocket(); + sinon.stub(adapter as any, 'createRokuDeploySocket').returns(fakeRokuDeploySocket); // Stub the settle method so connect() completes immediately sinon.stub(adapter as any, 'settleTelnetConnection').resolves(''); await adapter.connect(); // Wire up a promise that resolves when the adapter emits 'close'. This must be - // set up before the error is emitted, since Node.js auto-fires 'close' after - // 'error'. TelnetAdapter.emit() defers via setTimeout(0), so we await the - // promise rather than doing a fixed sleep. + // set up before the error is emitted, since a real socket always emits 'close' + // after a transport error. TelnetAdapter.emit() defers via setTimeout(0), so we + // await the promise rather than doing a fixed sleep. const closePromise = new Promise(resolve => { adapter.on('close', resolve); }); // The deferred is now resolved. Emitting an error on the still-live socket - // (simulating ETIMEDOUT on device disconnect) must not throw. + // (simulating ETIMEDOUT on device disconnect) must not throw, because the adapter + // is required to keep an 'error' listener attached to the socket at all times. expect(() => { - adapter['requestPipeline'].client.emit('error', new Error('read ETIMEDOUT')); + fakeRokuDeploySocket.emit('error', new Error('read ETIMEDOUT')); }).not.to.throw(); // On a real connected socket Node.js automatically fires 'close' after 'error'. - // Our test socket is never truly connected (connect is stubbed), so emit it manually - // to replicate that behaviour. - adapter['requestPipeline'].client.emit('close'); + // Our fake replicates that here. + fakeRokuDeploySocket.emit('close'); // TelnetAdapter.emit() wraps every event in setTimeout(0). Awaiting closePromise // yields back to the event loop so that deferred callback can run, confirming - // the full error → close → session-teardown chain fired correctly. + // the full error -> close -> session-teardown chain fired correctly. await closePromise; }); + + it('passes the configured device and port to the telnet socket factory', async () => { + const rceDevice = { instanceUrl: 'https://device.rce.roku.com/instance/abc', rceToken: 'token-value' }; + const rceAdapter = new TelnetAdapter( + { + device: rceDevice, + brightScriptConsolePort: 8085 + }, + rendezvousTracker + ); + sinon.stub(rokuDeploy, 'keyPress').resolves(); + sinon.stub(rceAdapter as any, 'settleTelnetConnection').resolves(''); + const fakeRokuDeploySocket = new FakeRokuDeploySocket(); + const createRokuDeploySocketStub = sinon.stub(rceAdapter as any, 'createRokuDeploySocket').returns(fakeRokuDeploySocket); + + await rceAdapter.connect(); + + expect(createRokuDeploySocketStub.calledOnce).to.be.true; + const options: SocketOptions = createRokuDeploySocketStub.firstCall.args[0]; + expect(options).to.eql({ + device: rceDevice, + port: 8085 + }); + }); + + it('passes the configured local device config to the telnet socket factory', async () => { + sinon.stub(rokuDeploy, 'keyPress').resolves(); + sinon.stub(adapter as any, 'settleTelnetConnection').resolves(''); + const fakeRokuDeploySocket = new FakeRokuDeploySocket(); + const createRokuDeploySocketStub = sinon.stub(adapter as any, 'createRokuDeploySocket').returns(fakeRokuDeploySocket); + + await adapter.connect(); + + expect(createRokuDeploySocketStub.calledOnce).to.be.true; + const options: SocketOptions = createRokuDeploySocketStub.firstCall.args[0]; + expect(options).to.eql({ + device: { host: '127.0.0.1' }, + port: 8085 + }); + }); }); }); diff --git a/src/adapters/TelnetAdapter.ts b/src/adapters/TelnetAdapter.ts index 70a47bfb..bc68086f 100644 --- a/src/adapters/TelnetAdapter.ts +++ b/src/adapters/TelnetAdapter.ts @@ -1,7 +1,7 @@ import { orderBy } from 'natural-orderby'; import * as EventEmitter from 'eventemitter3'; -import { Socket } from 'net'; -import { rokuDeploy } from 'roku-deploy'; +import { rokuDeploy, createRokuDeploySocket } from 'roku-deploy'; +import type { RokuDeploySocket, SocketOptions } from 'roku-deploy'; import { PrintedObjectParser } from '../PrintedObjectParser'; import type { BSDebugDiagnostic } from '../CompileErrorProcessor'; import { CompileErrorProcessor } from '../CompileErrorProcessor'; @@ -194,7 +194,7 @@ export class TelnetAdapter { * @param client * @param maxWaitMilliseconds */ - private settleTelnetConnection(client: Socket, maxWaitMilliseconds = 400) { + private settleTelnetConnection(client: RokuDeploySocket, maxWaitMilliseconds = 400) { const startTime = new Date(); this.logger.log('Waiting for telnet client to settle'); return new Promise((resolve) => { @@ -259,6 +259,14 @@ export class TelnetAdapter { return this.firstConnectDeferred.promise; } + /** + * Create the transport used to reach the device's BrightScript console. Extracted to a + * protected method so tests can substitute a fake socket. + */ + protected createRokuDeploySocket(options: SocketOptions): RokuDeploySocket { + return createRokuDeploySocket(options); + } + /** * Connect to the telnet session. This should be called before the channel is launched. */ @@ -268,14 +276,17 @@ export class TelnetAdapter { this.isInMicroDebugger = false; this.isNextBreakpointSkipped = false; try { + //normalizeAdapterOptions guarantees `device` is a concrete device config + const device = this.options.device; + this.logger.log('Pressing home button'); //force roku to return to home screen. This gives the roku adapter some security in knowing new messages won't be appearing during initialization - await rokuDeploy.pressHomeButton(this.options.host, this.options.remotePort); - let telnetSocket: Socket = new Socket({ allowHalfOpen: false }); - util.registerSocketLogging(telnetSocket, this.logger, 'TelnetSocket'); + await rokuDeploy.keyPress({ device: device, key: 'Home', ecpPort: this.options.remotePort }); + let socket = this.createRokuDeploySocket({ device: device, port: this.options.brightScriptConsolePort }); + util.registerSocketLogging(socket, this.logger, 'RokuDeploySocket'); //listen for the close event - telnetSocket.on('close', () => { + socket.on('close', () => { this.emit('close'); }); @@ -283,13 +294,13 @@ export class TelnetAdapter { //Use tryReject (not reject) because this handler persists for the socket's lifetime. //After a successful connection the deferred is already resolved, so a post-connection //socket error (e.g. ETIMEDOUT on device disconnect) must not crash the process. - telnetSocket.on('error', (err) => { - deferred.tryReject(new Error(`Error with connection to: ${this.options.host}:${this.options.brightScriptConsolePort} \n\n ${err.message} `)); + socket.on('error', (err) => { + deferred.tryReject(new Error(`Error with connection to: ${util.getDeviceLabel(device)}:${this.options.brightScriptConsolePort} \n\n ${err.message} `)); }); - const settlePromise = this.settleTelnetConnection(telnetSocket); - telnetSocket.connect(this.options.brightScriptConsolePort, this.options.host, () => { - this.logger.log(`Telnet connection established to ${this.options.host}:${this.options.brightScriptConsolePort}`); + const settlePromise = this.settleTelnetConnection(socket); + socket.connect(() => { + this.logger.log(`Telnet connection established to ${util.getDeviceLabel(device)}:${this.options.brightScriptConsolePort}`); this.connected = true; this.connectionDeferred.resolve(); this.emit('connected', this.connected); @@ -297,14 +308,14 @@ export class TelnetAdapter { const settledLogs = await settlePromise; if (settledLogs.trim().startsWith('Console connection is already in use.')) { - throw new SocketConnectionInUseError(`Telnet connection ${this.options.host}:${this.options.brightScriptConsolePort} already is use`, { + throw new SocketConnectionInUseError(`Telnet connection ${util.getDeviceLabel(device)}:${this.options.brightScriptConsolePort} already is use`, { port: this.options.brightScriptConsolePort, - host: this.options.host + host: util.getDeviceLabel(device) }); } //hook up the pipeline to the socket - this.requestPipeline = new TelnetRequestPipeline(telnetSocket); + this.requestPipeline = new TelnetRequestPipeline(socket); this.requestPipeline.connect(); let lastPartialLine = ''; diff --git a/src/adapters/TelnetRequestPipeline.ts b/src/adapters/TelnetRequestPipeline.ts index 992b0b3e..ffabbe0b 100644 --- a/src/adapters/TelnetRequestPipeline.ts +++ b/src/adapters/TelnetRequestPipeline.ts @@ -1,13 +1,13 @@ -import type { Socket } from 'net'; import * as EventEmitter from 'eventemitter3'; import { defer, util } from '../util'; import type { Logger } from '../logging'; import { createLogger } from '../logging'; import { Deferred } from 'brighterscript'; +import type { RokuDeploySocket } from 'roku-deploy'; export class TelnetRequestPipeline { public constructor( - public client: Socket + public client: RokuDeploySocket ) { } @@ -50,7 +50,7 @@ export class TelnetRequestPipeline { * Start listening for future incoming data from the client */ public connect() { - this.client.on('data', (data) => { + this.client.on('data', (data: Buffer) => { this.handleData(data.toString()); }); } diff --git a/src/debugProtocol/DebugProtocolClientReplaySession.ts b/src/debugProtocol/DebugProtocolClientReplaySession.ts index 54d8c6ea..ea10a5ce 100644 --- a/src/debugProtocol/DebugProtocolClientReplaySession.ts +++ b/src/debugProtocol/DebugProtocolClientReplaySession.ts @@ -84,7 +84,7 @@ export class DebugProtocolClientReplaySession { private createClient(controlPort: number) { this.client = new DebugProtocolClient({ controlPort: controlPort, - host: 'localhost' + device: { host: 'localhost' } }); //store the responses in the result diff --git a/src/debugProtocol/client/DebugProtocolClient.spec.ts b/src/debugProtocol/client/DebugProtocolClient.spec.ts index 408926a8..a1aeffaa 100644 --- a/src/debugProtocol/client/DebugProtocolClient.spec.ts +++ b/src/debugProtocol/client/DebugProtocolClient.spec.ts @@ -66,19 +66,14 @@ describe('DebugProtocolClient', () => { beforeEach(async () => { sinon.stub(console, 'log').callsFake((...args) => { }); - const options = { - controlPort: undefined as number, - host: '127.0.0.1' - }; + const controlPort = await util.getPort(); - if (!options.controlPort) { - options.controlPort = await util.getPort(); - } - server = new DebugProtocolServer(options); + //`host` is the DebugProtocolServer bind address; the client addresses its sockets by `device` + server = new DebugProtocolServer({ controlPort: controlPort, host: '127.0.0.1' }); plugin = server.plugins.add(new DebugProtocolServerTestPlugin()); await server.start(); - client = new DebugProtocolClient(options); + client = new DebugProtocolClient({ controlPort: controlPort, device: { host: '127.0.0.1' } }); //disable logging for tests because they clutter the test output client['logger'].logLevel = 'off'; }); diff --git a/src/debugProtocol/client/DebugProtocolClient.ts b/src/debugProtocol/client/DebugProtocolClient.ts index bac0f9f4..bdc5b817 100644 --- a/src/debugProtocol/client/DebugProtocolClient.ts +++ b/src/debugProtocol/client/DebugProtocolClient.ts @@ -1,4 +1,3 @@ -import * as Net from 'net'; import * as debounce from 'debounce'; import * as EventEmitter from 'eventemitter3'; import * as semver from 'semver'; @@ -48,6 +47,8 @@ import type { VerifiedBreakpoint } from '../events/updates/BreakpointVerifiedUpd import { BreakpointVerifiedUpdate } from '../events/updates/BreakpointVerifiedUpdate'; import type { AddConditionalBreakpointsResponse } from '../events/responses/AddConditionalBreakpointsResponse'; import { ExceptionBreakpointErrorUpdate } from '../events/updates/ExceptionBreakpointErrorUpdate'; +import { createRokuDeploySocket } from 'roku-deploy'; +import type { DeviceConfig, RokuDeploySocket, SocketOptions } from 'roku-deploy'; export class DebugProtocolClient { @@ -61,10 +62,9 @@ export class DebugProtocolClient { ) { this.options = { controlPort: 8081, - host: undefined, //override the defaults with the options from parameters ...options ?? {} - }; + } as ConstructorOptions; //add the internal plugin last, so it's the final plugin to handle the events this.addCorePlugin(); @@ -113,7 +113,7 @@ export class DebugProtocolClient { /** * The primary socket for this session. It's used to communicate with the debugger by sending commands and receives responses or updates */ - private controlSocket: Net.Socket; + private controlSocket: RokuDeploySocket; /** * Promise that is resolved when the control socket is closed */ @@ -121,7 +121,7 @@ export class DebugProtocolClient { /** * A socket where the debug server will send stdio */ - private ioSocket: Net.Socket; + private ioSocket: RokuDeploySocket; /** * Resolves when the ioSocket has closed */ @@ -195,18 +195,29 @@ export class DebugProtocolClient { this.emitter.emit(eventName, data); } + /** + * Create the transport used to reach one of the device's debug protocol ports (the control port + * or the io port): a raw tcp socket for a local device, or the RCE instance api's + * `/api/v0/ports/` WebSocket for a cloud device. Extracted to a protected method so tests + * can substitute a fake socket. + */ + protected createRokuDeploySocket(options: SocketOptions): RokuDeploySocket { + return createRokuDeploySocket(options); + } + /** * A collection of sockets created when trying to connect to the debug protocol's control socket. We keep these around for quicker tear-down * whenever there is an early-terminated debug session */ private async establishControlConnection() { - const connection = await new Promise((resolve) => { - const socket = new Net.Socket({ - allowHalfOpen: false + const connection = await new Promise((resolve) => { + const socket = this.createRokuDeploySocket({ + device: this.options.device, + port: this.options.controlPort }); util.registerSocketLogging(socket, this.logger, 'ControlSocket'); - socket.connect({ port: this.options.controlPort, host: this.options.host }, () => { + socket.connect(() => { resolve(socket); }); }); @@ -232,7 +243,7 @@ export class DebugProtocolClient { // If there is no error, the server has accepted the request and created a new dedicated control socket this.controlSocket = await this.establishControlConnection(); - this.controlSocket.on('data', (data) => { + this.controlSocket.on('data', (data: Buffer) => { this.writeToBufferLog('server-to-client', data); this.emit('data', data); //queue up processing the new data, chunk by chunk @@ -1079,27 +1090,25 @@ export class DebugProtocolClient { */ private connectToIoPort(update: IOPortOpenedUpdate) { if (update.success) { - // Create a new TCP client. - this.ioSocket = new Net.Socket({ - allowHalfOpen: false + // Create a new client socket to the io port the device just opened + this.ioSocket = this.createRokuDeploySocket({ + device: this.options.device, + port: update.data.port }); util.registerSocketLogging(this.ioSocket, this.logger, 'IoSocket'); // Send a connection request to the server. - this.logger.log(`Connect to IO Port ${this.options.host}:${update.data.port}`); + this.logger.log(`Connect to IO Port ${update.data.port}`); //sometimes the server shuts down before we had a chance to connect, so recover more gracefully try { - this.ioSocket.connect({ - port: update.data.port, - host: this.options.host - }, () => { + this.ioSocket.connect(() => { // If there is no error, the server has accepted the request this.logger.log('TCP connection established with the IO Port.'); this.connectedToIoPort = true; let lastPartialLine = ''; - this.ioSocket.on('data', (buffer) => { + this.ioSocket.on('data', (buffer: Buffer) => { this.writeToBufferLog('io', buffer); let logResult = util.handleLogFragments(lastPartialLine, buffer.toString()); @@ -1126,7 +1135,7 @@ export class DebugProtocolClient { }); return true; } catch (e) { - this.logger.error(`Failed to connect to IO socket at ${this.options.host}:${update.data.port}`, e); + this.logger.error(`Failed to connect to IO socket at port ${update.data.port}`, e); this.emit('app-exit'); } } @@ -1274,9 +1283,12 @@ export interface BreakpointSpec { export interface ConstructorOptions { /** - * The host/ip address of the Roku + * The roku-deploy device config for the target device. This is the only way this client + * addresses the device: a local device connects raw tcp sockets to its debug protocol ports, + * and a Roku Cloud Emulator device reaches the same ports through its instance api's + * `/api/v0/ports/` WebSocket routes. */ - host: string; + device: DeviceConfig; /** * The port number used to send all debugger commands. This is static/unchanging for Roku devices, * but is configurable here to support unit testing or alternate runtimes (i.e. https://www.npmjs.com/package/brs) diff --git a/src/debugProtocol/client/DebugProtocolClientPlugin.ts b/src/debugProtocol/client/DebugProtocolClientPlugin.ts index aab44459..7a08f288 100644 --- a/src/debugProtocol/client/DebugProtocolClientPlugin.ts +++ b/src/debugProtocol/client/DebugProtocolClientPlugin.ts @@ -1,5 +1,5 @@ import type { DebugProtocolClient } from './DebugProtocolClient'; -import type { Socket } from 'net'; +import type { RokuDeploySocket } from 'roku-deploy'; import type { ProtocolRequest, ProtocolResponse, ProtocolUpdate } from '../events/ProtocolEvent'; export interface DebugProtocolClientPlugin { @@ -18,7 +18,7 @@ export interface DebugProtocolClientPlugin { export interface OnServerConnectedEvent { client: DebugProtocolClient; - server: Socket; + server: RokuDeploySocket; } export interface ProvideResponseOrUpdateEvent { diff --git a/src/debugSession/BrightScriptDebugSession.spec.ts b/src/debugSession/BrightScriptDebugSession.spec.ts index 9cbdf186..652bc20f 100644 --- a/src/debugSession/BrightScriptDebugSession.spec.ts +++ b/src/debugSession/BrightScriptDebugSession.spec.ts @@ -12,7 +12,7 @@ import type { StackFrame } from '../adapters/TelnetAdapter'; import { PrimativeType, TelnetAdapter } from '../adapters/TelnetAdapter'; import { defer, util } from '../util'; import { HighLevelType } from '../interfaces'; -import type { LaunchConfiguration } from '../LaunchConfiguration'; +import type { LaunchConfiguration, ResolvedLaunchConfiguration } from '../LaunchConfiguration'; import type { SinonStub } from 'sinon'; import { DiagnosticSeverity, util as bscUtil, standardizePath as s } from 'brighterscript'; import { CompileError, DefaultFiles, rokuDeploy } from 'roku-deploy'; @@ -48,7 +48,7 @@ describe('BrightScriptDebugSession', () => { let session: BrightScriptDebugSession; - let launchConfiguration: LaunchConfiguration; + let launchConfiguration: ResolvedLaunchConfiguration; let initRequestArgs: DebugProtocol.InitializeRequestArguments; let rokuAdapter: TelnetAdapter; @@ -82,7 +82,8 @@ describe('BrightScriptDebugSession', () => { rootDir: rootDir, outDir: outDir, stagingDir: stagingDir, - files: DefaultFiles + files: DefaultFiles, + device: { host: '1.2.3.4' } } as any; session['launchConfiguration'] = launchConfiguration; session.projectManager.launchConfiguration = launchConfiguration; @@ -92,26 +93,24 @@ describe('BrightScriptDebugSession', () => { //mock the rokuDeploy module with promises so we can have predictable tests session.rokuDeploy = { - prepublishToStaging: () => { - return Promise.resolve(); + withDnsResolvedHost: (device) => { + return Promise.resolve(device); }, - zipPackage: () => { + stage: () => { return Promise.resolve(); }, - pressHomeButton: () => { + zip: () => { return Promise.resolve(); }, - publish: () => { + keyPress: () => { return Promise.resolve(); }, - createPackage: () => { + sideload: () => { return Promise.resolve(); }, - deploy: () => { + createSignedPackage: () => { return Promise.resolve(); }, - getOptions: () => { - }, getFilePaths: () => { } }; @@ -204,11 +203,13 @@ describe('BrightScriptDebugSession', () => { let sendEvent = session.sendEvent.bind(session); sinon.stub(session, 'sendEvent').callsFake((event) => { if (isCustomRequestEvent(event)) { - void rokuDeploy.zipFolder(session['launchConfiguration'].stagingDir, packagePath).then(() => { + void rokuDeploy.zip({ dir: session['launchConfiguration'].stagingDir, out: packagePath }).then(() => { //pretend we are the client and send a response back session.emit(ClientToServerCustomEventName.customRequestEventResponse, { requestId: event.body.requestId }); + }, (e) => { + console.error('Failed to zip the staging folder', e); }); } else { //call through @@ -221,7 +222,7 @@ describe('BrightScriptDebugSession', () => { return Promise.resolve(session['rokuAdapter']); }); - const publishStub = sinon.stub(session.rokuDeploy, 'publish').callsFake(() => { + const publishStub = sinon.stub(session.rokuDeploy, 'sideload').callsFake(() => { //emit the app-ready event (session['rokuAdapter'] as TelnetAdapter)['emit']('app-ready'); @@ -233,8 +234,10 @@ describe('BrightScriptDebugSession', () => { await session.launchRequest({} as any, { cwd: tempDir, + device: { host: '1.2.3.4' }, //where the source files reside rootDir: rootDir, + files: DefaultFiles, //where roku-debug should put the staged files (and inject breakpoints) stagingDir: `${stagingDir}/staging`, //the name of the task that should be run to create the zip (doesn't matter for this test...we're going to intercept it anyway) @@ -1257,6 +1260,29 @@ describe('BrightScriptDebugSession', () => { expect(events.filter(e => e instanceof ProgressEndEvent)).to.be.empty; }); + + it('flushes the deferred ProgressEndEvent when shutdown lands inside the end delay', async () => { + const clock = sinon.useFakeTimers(); + const events = []; + sinon.stub(session, 'sendEvent').callsFake((event) => events.push(event)); + + session['initRequestArgs'].supportsProgressReporting = true; + session['sendLaunchProgress']('start', 'Waiting on application'); + session['sendLaunchProgress']('end', 'Complete'); + //the end event is held back for UX, so it has not been sent yet + expect(events.filter(e => e instanceof ProgressEndEvent)).to.be.empty; + + const shutdownPromise = session.shutdown(); + //the flush happens synchronously at the start of shutdown, before the delay elapses, + //so the notification cannot get stuck open when the adapter exits + expect(events.filter(e => e instanceof ProgressEndEvent)).to.have.lengthOf(1); + + await clock.tickAsync(2000); + await shutdownPromise; + + //the cancelled delay timer must not deliver a second end event + expect(events.filter(e => e instanceof ProgressEndEvent)).to.have.lengthOf(1); + }); }); describe('disconnectRequest', () => { @@ -1264,7 +1290,7 @@ describe('BrightScriptDebugSession', () => { // - https://github.com/rokucommunity/vscode-brightscript-language/issues/807 (EHOSTDOWN) // - https://github.com/rokucommunity/roku-debug/issues/332 (ECONNREFUSED) //@vscode/debugadapter dispatches disconnectRequest without awaiting the returned Promise - //(debugSession.js:391), so any rejection from `await this.rokuDeploy.pressHomeButton(...)` + //(debugSession.js:391), so any rejection from the `await this.rokuDeploy.keyPress(...)` home press //becomes an unhandled rejection that crashes the DAP process. When the device is powered //off / unreachable at disconnect time, the ECP connect attempt fails — the specific Node //error code depends on the OS-level reason (host unresponsive vs. connection refused). @@ -1275,7 +1301,7 @@ describe('BrightScriptDebugSession', () => { host: '192.168.1.17', remotePort: 8060 }; - session.rokuDeploy.pressHomeButton = () => Promise.reject(rejection); + session.rokuDeploy.keyPress = () => Promise.reject(rejection); //stub shutdown so the test doesn't tear down the whole session machinery sinon.stub(session, 'shutdown').resolves(); } @@ -1881,7 +1907,7 @@ describe('BrightScriptDebugSession', () => { sinon.stub(ComponentLibraryProject.prototype, 'postfixFiles').resolves(); sinon.stub(ComponentLibraryProject.prototype, 'zipPackage').resolves(); sinon.stub(session.projectManager, 'applyLibraryReferencePostfixes').resolves(); - session['launchConfiguration'].host = '192.168.1.100'; + session['launchConfiguration'].device = { host: '192.168.1.100' }; session['launchConfiguration'].password = 'test123'; session.projectManager.mainProject = { stagingDir: s`${tempDir}/main-staging`, @@ -1892,8 +1918,8 @@ describe('BrightScriptDebugSession', () => { it('installs libraries sequentially when marked install=true', async () => { stubDefaults(); const installOrder = []; - const publishStub = sinon.stub(rokuDeploy, 'publish').callsFake(async (options) => { - installOrder.push(options.outFile); + const publishStub = sinon.stub(rokuDeploy, 'sideload').callsFake(async (options) => { + installOrder.push(path.basename(options.zip)); await util.sleep(10); return { message: 'success', results: [] }; }); @@ -1910,8 +1936,8 @@ describe('BrightScriptDebugSession', () => { it('skips libraries where install is not true', async () => { stubDefaults(); const installOrder = []; - const publishStub = sinon.stub(rokuDeploy, 'publish').callsFake(async (options) => { - installOrder.push(options.outFile); + const publishStub = sinon.stub(rokuDeploy, 'sideload').callsFake(async (options) => { + installOrder.push(path.basename(options.zip)); await util.sleep(10); return { message: 'success', results: [] }; }); @@ -1930,24 +1956,25 @@ describe('BrightScriptDebugSession', () => { it('sends proper form data for installation', async () => { stubDefaults(); - const publishStub = sinon.stub(rokuDeploy, 'publish').resolves({ message: 'success', results: [] }); + const publishStub = sinon.stub(rokuDeploy, 'sideload').resolves({ message: 'success', results: [] }); await runPrepareAndHost([ { rootDir: complib1Dir, outFile: 'testLib.zip', install: true } ] as any, 8080); - expect(publishStub.getCall(0).args[0]).to.include({ - host: '192.168.1.100', + const options = publishStub.getCall(0).args[0]; + expect(options).to.include({ password: 'test123', username: 'rokudev', - outFile: 'testLib.zip', appType: 'dcl' }); + expect(options.device).to.eql({ host: '192.168.1.100' }); + expect(path.basename(options.zip)).to.equal('testLib.zip'); }); it('logs the error AND fails the launch when a library install fails', async () => { stubDefaults(); - sinon.stub(rokuDeploy, 'publish').rejects(new Error('Network error')); + sinon.stub(rokuDeploy, 'sideload').rejects(new Error('Network error')); //a failed install must abort the launch (not silently continue with a missing library) await expectThrowsAsync(() => runPrepareAndHost([ @@ -1964,7 +1991,7 @@ describe('BrightScriptDebugSession', () => { sinon.stub(session['componentLibraryServer'], 'startStaticFileHosting').resolves(); sinon.stub(ComponentLibraryProject.prototype, 'postfixFiles').resolves(); sinon.stub(session.projectManager, 'applyLibraryReferencePostfixes').resolves(); - session['launchConfiguration'].host = '192.168.1.100'; + session['launchConfiguration'].device = { host: '192.168.1.100' }; session['launchConfiguration'].password = 'test123'; session.projectManager.mainProject = { stagingDir: s`${tempDir}/main-staging`, @@ -1982,8 +2009,8 @@ describe('BrightScriptDebugSession', () => { events.push(`zip-${this['outFile']}`); await util.sleep(1); }); - sinon.stub(rokuDeploy, 'publish').callsFake((options) => { - events.push(`install-${options.outFile}`); + sinon.stub(rokuDeploy, 'sideload').callsFake((options) => { + events.push(`install-${path.basename(options.zip)}`); return Promise.resolve({ message: 'success', results: [] }); }); @@ -2004,7 +2031,7 @@ describe('BrightScriptDebugSession', () => { sinon.stub(session['componentLibraryServer'], 'startStaticFileHosting').resolves(); sinon.stub(ComponentLibraryProject.prototype, 'postfixFiles').resolves(); sinon.stub(ComponentLibraryProject.prototype, 'zipPackage').resolves(); - session['launchConfiguration'].host = '192.168.1.100'; + session['launchConfiguration'].device = { host: '192.168.1.100' }; session['launchConfiguration'].password = 'test123'; sinon.stub(ComponentLibraryProject.prototype, 'stage').rejects(new Error('Stage failed')); @@ -2048,7 +2075,7 @@ describe('BrightScriptDebugSession', () => { it('calls packageTask for each component library if packageTask defined', async () => { stubDefaults(); - sinon.stub(rokuDeploy, 'publish').resolves({ message: 'success', results: [] }); + sinon.stub(rokuDeploy, 'sideload').resolves({ message: 'success', results: [] }); const sendEventStub = sinon.stub(session as any, 'sendCustomRequest').resolves(); await runPrepareAndHost([ @@ -2068,7 +2095,7 @@ describe('BrightScriptDebugSession', () => { it('handles packagePath and packageUploadOverrides for component libraries', async () => { stubDefaults(); const installOrder = []; - sinon.stub(rokuDeploy, 'publish').callsFake(async (options) => { + sinon.stub(rokuDeploy, 'sideload').callsFake(async (options) => { installOrder.push(options); await util.sleep(10); return { message: 'success', results: [] }; @@ -2102,13 +2129,11 @@ describe('BrightScriptDebugSession', () => { expect(installOrder.length).to.equal(2); expect(installOrder[0]).to.include({ - outFile: path.basename(s`${tempDir}/custom/cl1.zip`), - outDir: path.dirname(s`${tempDir}/custom/cl1.zip`), + zip: s`${tempDir}/custom/cl1.zip`, packageUploadOverrides: packageUploadOverrides1 }); expect(installOrder[1]).to.include({ - outFile: path.basename(s`${tempDir}/custom/cl2.zip`), - outDir: path.dirname(s`${tempDir}/custom/cl2.zip`), + zip: s`${tempDir}/custom/cl2.zip`, packageUploadOverrides: packageUploadOverrides2 }); }); @@ -2131,7 +2156,7 @@ describe('BrightScriptDebugSession', () => { const present = new Set(installed); const deleteOrder: string[] = []; - session['launchConfiguration'].host = '192.168.1.100'; + session['launchConfiguration'].device = { host: '192.168.1.100' }; session['launchConfiguration'].password = 'test123'; //deletion pauses/resumes compile-error reporting on the adapter; stub those so the flow works without a device session['rokuAdapter'] = {}; @@ -2280,7 +2305,7 @@ describe('BrightScriptDebugSession', () => { it('re-throws a non-compile error (e.g. auth/network failure) immediately', async () => { configureComplibs(['LibAlpha.zip']); - session['launchConfiguration'].host = '192.168.1.100'; + session['launchConfiguration'].device = { host: '192.168.1.100' }; sinon.stub(rokuDeploy, 'listSideloadedPlugins').resolves([{ appType: 'dcl', archiveFileName: 'LibAlpha.zip' } as any]); sinon.stub(rokuDeploy, 'deleteComponentLibrary').rejects(new Error('Unauthorized. Please verify credentials')); @@ -2292,7 +2317,7 @@ describe('BrightScriptDebugSession', () => { configureComplibs([]); //a complib that always fails with a compile error and is never unblocked - should give up and throw sinon.stub(rokuDeploy, 'listSideloadedPlugins').resolves([{ appType: 'dcl', archiveFileName: 'stuck.zip' } as any]); - session['launchConfiguration'].host = '192.168.1.100'; + session['launchConfiguration'].device = { host: '192.168.1.100' }; sinon.stub(rokuDeploy, 'deleteComponentLibrary').rejects(new Error('Install Failure: Compilation Failed. (compile error &hb9)')); await expectThrowsAsync(() => session['deleteAllComponentLibraries']()); @@ -3006,7 +3031,7 @@ describe('BrightScriptDebugSession', () => { //stub PerfettoManager prototype methods so no real connections are made startTracingStub = sinon.stub(PerfettoManager.prototype, 'startTracing').resolves(); sinon.stub(PerfettoManager.prototype, 'on').returns(() => { }); - session['perfettoManager'] = new PerfettoManager({ host: 'localhost' }); + session['perfettoManager'] = new PerfettoManager({ device: { host: 'localhost' } }); }); it('calls startTracing when connectOnStart is true and device supports perfetto', async () => { @@ -3183,7 +3208,9 @@ describe('BrightScriptDebugSession', () => { setupLaunchStubs(); const getDeviceInfoStub = rokuDeploy.getDeviceInfo as sinon.SinonStub; - launchConfiguration.host = '1.2.3.4'; + //the deprecated dap-input path: launchRequest normalizes this into the device config + delete (launchConfiguration as any).device; + (launchConfiguration as LaunchConfiguration).host = '1.2.3.4'; launchConfiguration.deviceInfo = { 'developer-enabled': 'true', 'software-version': '11.5.0', @@ -3207,7 +3234,9 @@ describe('BrightScriptDebugSession', () => { setupLaunchStubs(); const getDeviceInfoStub = rokuDeploy.getDeviceInfo as sinon.SinonStub; - launchConfiguration.host = '1.2.3.4'; + //the deprecated dap-input path: launchRequest normalizes this into the device config + delete (launchConfiguration as any).device; + (launchConfiguration as LaunchConfiguration).host = '1.2.3.4'; await session.launchRequest({} as any, launchConfiguration); @@ -3276,7 +3305,7 @@ describe('BrightScriptDebugSession', () => { setupLaunchStubs(); // Override the publish stub to throw a CompileError (session as any).publish.restore(); - sinon.stub(session as any, 'publish').rejects(new CompileError('compile failed', [], {} as any)); + sinon.stub(session as any, 'publish').rejects(new CompileError('compile failed')); session['initRequestArgs'].supportsProgressReporting = true; await session.launchRequest({} as any, launchConfiguration); @@ -3314,7 +3343,7 @@ describe('BrightScriptDebugSession', () => { sinon.stub(session as any, 'createRokuAdapter').callsFake(() => { }); sinon.stub(session as any, 'runAutomaticSceneGraphCommands').resolves(); sinon.stub(session as any, 'tryProfilingConnectOnStart').resolves(); - sinon.stub(session.rokuDeploy, 'pressHomeButton').resolves(); + sinon.stub(session.rokuDeploy, 'keyPress').resolves(); sinon.stub(session as any, 'publish').resolves(); sinon.stub(session, 'setupProcessErrorHandlers'); //staging is a no-op for this test; we care about the write/postfix -> rewrite -> zip ordering @@ -3392,6 +3421,72 @@ describe('BrightScriptDebugSession', () => { }); }); + describe('device option', () => { + it('reads the device config from the launch config', () => { + (session as any).launchConfiguration = { device: { host: '1.2.3.4' } }; + expect(session['launchConfiguration'].device).to.eql({ host: '1.2.3.4' }); + expect(session['deviceLabel']).to.equal('1.2.3.4'); + }); + + it('builds the device option from the deprecated host field during normalize', () => { + const config = session['normalizeLaunchConfig']({ host: '1.2.3.4' } as any); + expect(config.device).to.eql({ host: '1.2.3.4' }); + }); + + it('deletes the deprecated host field from the config during normalize', () => { + const config = session['normalizeLaunchConfig']({ host: '1.2.3.4' } as any); + expect('host' in config).to.be.false; + }); + + it('prefers a supplied device config over the deprecated host field during normalize', () => { + const config = session['normalizeLaunchConfig']({ device: { host: '5.6.7.8' }, host: '1.2.3.4' } as any); + (session as any).launchConfiguration = config; + expect(session['launchConfiguration'].device).to.eql({ host: '5.6.7.8' }); + }); + + it('aborts the launch with a clear message when the config supplies no device addressing', async () => { + const shutdownStub = sinon.stub(session, 'shutdown').resolves(); + + await session.launchRequest({} as any, {} as any); + + expect(shutdownStub.calledOnce).to.be.true; + expect(shutdownStub.getCall(0).args[0]).to.include('does not specify a target device'); + }); + + it('passes a cloud emulator device config through untouched and never leaks the token in the label', () => { + const device = { instanceUrl: 'https://device.rce.roku.com/instance/abc', rceToken: 'secret' }; + (session as any).launchConfiguration = { device: device }; + expect(session['launchConfiguration'].device).to.equal(device); + expect(session['deviceLabel']).to.equal('https://device.rce.roku.com/instance/abc'); + expect(session['deviceLabel']).not.to.include('secret'); + }); + + it('labels id-addressed and esn-addressed cloud emulator devices by their identifier', () => { + (session as any).launchConfiguration = { device: { id: 83, rceToken: 'secret' } }; + expect(session['deviceLabel']).to.equal('83'); + (session as any).launchConfiguration = { device: { esn: 'XY020078HH5S', rceToken: 'secret' } }; + expect(session['deviceLabel']).to.equal('XY020078HH5S'); + }); + + it('sends the launch config device to sideload', async () => { + const device = { instanceUrl: 'https://device.rce.roku.com/instance/abc', rceToken: 'secret' }; + (session as any).launchConfiguration = { + ...session['launchConfiguration'], + device: device, + outDir: tempDir + }; + rokuAdapter.connected = true; + const sideloadStub = sinon.stub(session.rokuDeploy, 'sideload').callsFake(() => { + (session['rokuAdapter'] as TelnetAdapter)['emit']('app-ready'); + return Promise.resolve({ message: 'success', results: [] }); + }); + + await (session as any).publish(); + + expect(sideloadStub.getCall(0).args[0].device).to.equal(device); + }); + }); + describe('publish', () => { it('waits 60 seconds before aborting when the app never becomes ready', async () => { session['publishTimeout'] = 60_000; @@ -3399,7 +3494,7 @@ describe('BrightScriptDebugSession', () => { const clock = sinon.useFakeTimers(); const shutdownStub = sinon.stub(session, 'shutdown').resolves() as unknown as SinonStub; rokuAdapter.connected = false; - sinon.stub(session.rokuDeploy, 'publish').resolves(); + sinon.stub(session.rokuDeploy, 'sideload').resolves(); const publishPromise = (session as any).publish(); diff --git a/src/debugSession/BrightScriptDebugSession.ts b/src/debugSession/BrightScriptDebugSession.ts index 849e314b..f0ffbca2 100644 --- a/src/debugSession/BrightScriptDebugSession.ts +++ b/src/debugSession/BrightScriptDebugSession.ts @@ -3,7 +3,7 @@ import { orderBy } from 'natural-orderby'; import * as path from 'path'; import * as semver from 'semver'; import { rokuDeploy, CompileError, isUpdateCheckRequiredError, isConnectionResetError, EcpNetworkAccessModeDisabledError } from 'roku-deploy'; -import type { DeviceInfo, RokuDeploy, RokuDeployOptions } from 'roku-deploy'; +import type { DeviceInfo, RokuDeploy, SideloadOptions } from 'roku-deploy'; import { BreakpointEvent, LoggingDebugSession, @@ -58,10 +58,11 @@ import { FileManager } from '../managers/FileManager'; import { SourceMapManager } from '../managers/SourceMapManager'; import { LocationManager } from '../managers/LocationManager'; import type { AugmentedSourceBreakpoint } from '../managers/BreakpointManager'; +import type { ResolvedLaunchConfiguration } from '../LaunchConfiguration'; import { BreakpointManager } from '../managers/BreakpointManager'; import type { LogMessage } from '../logging'; import { PerfettoManager } from '../PerfettoManager'; -import { logger, FileLoggingManager, debugServerLogOutputEventTransport, LogLevelPriority } from '../logging'; +import { logger, FileLoggingManager, debugServerLogOutputEventTransport } from '../logging'; import { VariableType } from '../debugProtocol/events/responses/VariablesResponse'; import { DiagnosticSeverity } from 'brighterscript'; import type { ExceptionBreakpoint } from '../debugProtocol/events/requests/SetExceptionBreakpointsRequest'; @@ -358,6 +359,14 @@ export class BrightScriptDebugSession extends LoggingDebugSession { //set imports as class properties so they can be spied upon during testing public rokuDeploy = rokuDeploy as unknown as RokuDeploy; + /** + * A short human-readable identifier for the target device, safe for log and error messages + * (never includes credentials like the rceToken) + */ + private get deviceLabel(): string { + return util.getDeviceLabel(this.launchConfiguration.device); + } + private componentLibraryServer = new ComponentLibraryServer(); private rokuAdapterDeferred = defer(); @@ -397,6 +406,14 @@ export class BrightScriptDebugSession extends LoggingDebugSession { */ private launchProgressId: string | undefined; + /** + * Sends the deferred ProgressEndEvent for the launch progress bar (sendLaunchProgress holds it + * back for UX). Kept here so shutdown can flush it immediately: the adapter can exit before the + * delay elapses, which would otherwise leave the client's progress notification stuck open. + * Cleared once the event has been sent. + */ + private flushLaunchProgressEnd: (() => void) | undefined; + /** * The first encountered compile error, will be used to send to the client as a runtime error (nicer UI presentation) */ @@ -428,7 +445,13 @@ export class BrightScriptDebugSession extends LoggingDebugSession { return this.rokuAdapter; } - private launchConfiguration: LaunchConfiguration; + /** + * The normalized launch config for this session. The resolved type has no `host` (so nothing in + * the session can read or write it) and a concrete `device` config, which is the only way the + * debugger addresses the device. The raw `LaunchConfiguration` exists only as launchRequest's + * DAP input; normalizeLaunchConfig converts it. + */ + private launchConfiguration: ResolvedLaunchConfiguration; private initRequestArgs: DebugProtocol.InitializeRequestArguments; private exceptionBreakpoints: ExceptionBreakpoint[] = []; @@ -596,10 +619,24 @@ export class BrightScriptDebugSession extends LoggingDebugSession { * @param config * @returns */ - private normalizeLaunchConfig(config: LaunchConfiguration) { + private normalizeLaunchConfig(config: LaunchConfiguration): ResolvedLaunchConfiguration { + //`device` is the canonical way to address the target device; `host` is a deprecated alias. + //this is the ONLY place the debugger reads the top-level `host` field: whatever was supplied + //is resolved to a concrete device config here, and everything downstream uses `device`. + if (!config.device && config.host) { + config.device = { host: config.host }; + } + //the deprecated field is now consumed. Delete it so the runtime object matches the resolved + //type and nothing downstream (including the configs echoed back to the client) carries it. + delete config.host; + //an RCE device config without a token picks one up from the environment (the extension + //injects ROKU_RCE_TOKEN into this process so the token does not have to travel through the + //launch config over DAP). The custom events that echo this config back to the client scrub + //the token out again (see Events.ts), so it never rides the DAP wire in either direction. + config.device = util.hydrateRceTokenFromEnv(config.device); config.cwd ??= process.cwd(); config.outDir ??= s`${config.cwd}/out`; - config.stagingDir ??= s`${config.outDir}/.roku-deploy-staging`; + config.stagingDir ??= util.getStagingDir({ outDir: config.outDir, cwd: config.cwd }); config.componentLibrariesPort ??= 8080; config.packagePort ??= 80; config.remotePort ??= 8060; @@ -623,7 +660,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { config.enableVariablesPanel = true; } config.deferScopeLoading ??= config.enableVariablesPanel === false; - return config; + return config as ResolvedLaunchConfiguration; } public async launchRequest(response: DebugProtocol.LaunchResponse, config: LaunchConfiguration) { @@ -632,6 +669,11 @@ export class BrightScriptDebugSession extends LoggingDebugSession { try { this.resetSessionState(); this.launchConfiguration = this.normalizeLaunchConfig(config); + //fail fast when the launch config supplied no device addressing at all, rather than + //failing later with a confusing dns or connection error for an undefined host + if (!this.launchConfiguration.device) { + return await this.shutdown(`Launch config does not specify a target device. Please supply the 'device' option (or the deprecated 'host' option).`); + } this.setupProcessErrorHandlers(); //prebake some threads for our ProjectManager to use later on (1 for the main project, and 1 for every complib) @@ -644,11 +686,12 @@ export class BrightScriptDebugSession extends LoggingDebugSession { this.sendLaunchProgress('start', 'Finding device on network'); - //do a DNS lookup for the host to fix issues with roku rejecting ECP + //do a DNS lookup for the host to fix issues with roku rejecting ECP. Only local devices + //are addressed by host; other device types (like the Roku Cloud Emulator) pass through unchanged try { - this.launchConfiguration.host = await util.dnsLookup(this.launchConfiguration.host); + this.launchConfiguration.device = await this.rokuDeploy.withDnsResolvedHost(this.launchConfiguration.device); } catch (e) { - return this.shutdown(`Could not resolve ip address for host '${this.launchConfiguration.host}'`); + return this.shutdown(`Could not resolve ip address for host '${this.deviceLabel}'`); } // fetch device info if not supplied via launch config @@ -656,20 +699,20 @@ export class BrightScriptDebugSession extends LoggingDebugSession { if (this.launchConfiguration.deviceInfo) { this.deviceInfo = rokuDeploy.enhanceDeviceInfo(this.launchConfiguration.deviceInfo); } else { - this.deviceInfo = await rokuDeploy.getDeviceInfo({ host: this.launchConfiguration.host, remotePort: this.launchConfiguration.remotePort, enhance: true, timeout: 4_000 }); + this.deviceInfo = await rokuDeploy.getDeviceInfo({ device: this.launchConfiguration.device, ecpPort: this.launchConfiguration.remotePort, enhance: true, timeout: 4_000 }); } if (this.deviceInfo.ecpSettingMode === 'limited') { - return await this.shutdown(`To allow the debugger to communicate properly, please ensure on the Roku device that 'Settings' > 'System' > 'Advanced system settings' > 'Control by mobile apps' is set to "Enabled" or "Permissive". Current mode: Limited (device: ${this.launchConfiguration.host})`); + return await this.shutdown(`To allow the debugger to communicate properly, please ensure on the Roku device that 'Settings' > 'System' > 'Advanced system settings' > 'Control by mobile apps' is set to "Enabled" or "Permissive". Current mode: Limited (device: ${this.deviceLabel})`); } } catch (e) { if (e instanceof EcpNetworkAccessModeDisabledError) { - return this.shutdown(`To allow the debugger to communicate properly, please ensure on the Roku device that 'Settings' > 'System' > 'Advanced system settings' > 'Control by mobile apps' is set to "Enabled" or "Permissive". Current mode: Disabled (device: ${this.launchConfiguration.host})`); + return this.shutdown(`To allow the debugger to communicate properly, please ensure on the Roku device that 'Settings' > 'System' > 'Advanced system settings' > 'Control by mobile apps' is set to "Enabled" or "Permissive". Current mode: Disabled (device: ${this.deviceLabel})`); } - return this.shutdown(`Unable to connect to roku at '${this.launchConfiguration.host}'. Verify the IP address is correct and that the device is powered on and connected to same network as this computer.`); + return this.shutdown(`Unable to connect to roku at '${this.deviceLabel}'. Verify the device address is correct and that the device is powered on and reachable.`); } if (this.deviceInfo && !this.deviceInfo.developerEnabled) { - return await this.shutdown(`Developer mode is not enabled for host '${this.launchConfiguration.host}'.`); + return await this.shutdown(`Developer mode is not enabled for device '${this.deviceLabel}'.`); } // everything is ready, send the response to the launch request so the UI can update and configuration can begin @@ -705,9 +748,9 @@ export class BrightScriptDebugSession extends LoggingDebugSession { packageEnd(); if (this.enableDebugProtocol) { - util.log(`Connecting to Roku via the BrightScript debug protocol at ${this.launchConfiguration.host}:${this.launchConfiguration.controlPort}`); + util.log(`Connecting to Roku via the BrightScript debug protocol at ${this.deviceLabel}:${this.launchConfiguration.controlPort}`); } else { - util.log(`Connecting to Roku via telnet at ${this.launchConfiguration.host}:${this.launchConfiguration.brightScriptConsolePort}`); + util.log(`Connecting to Roku via telnet at ${this.deviceLabel}:${this.launchConfiguration.brightScriptConsolePort}`); } //activate rendezvous tracking (if enabled). Log the error and move on if it crashes, this shouldn't bring down the session. @@ -728,7 +771,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { if (this.launchConfiguration.componentLibraries?.some(x => x.install)) { this.sendLaunchProgress('update', 'Removing existing dev app and component libraries'); await rokuDeploy.deleteAllSideloadedPlugins({ - host: this.launchConfiguration.host, + device: this.launchConfiguration.device, password: this.launchConfiguration.password }); } @@ -786,7 +829,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { await this.runAutomaticSceneGraphCommands(this.launchConfiguration.autoRunSgDebugCommands); //press the home button to ensure we're at the home screen - await this.rokuDeploy.pressHomeButton(this.launchConfiguration.host, this.launchConfiguration.remotePort); + await this.rokuDeploy.keyPress({ device: this.launchConfiguration.device, key: 'Home', ecpPort: this.launchConfiguration.remotePort }); //pass the log level down thought the adapter to the RendezvousTracker and ChanperfTracker this.rokuAdapter.setConsoleOutput(this.launchConfiguration.consoleOutput); @@ -804,7 +847,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { this.rokuAdapter.on('device-unresponsive', async (data: { lastCommand: string }) => { const stopDebuggerAction = 'Stop Debugger'; - const message = `Roku device ${this.launchConfiguration.host} is not responding and may not recover.` + + const message = `Roku device ${this.deviceLabel} is not responding and may not recover.` + (data.lastCommand ? `\n\nActive command:\n"${util.truncate(data.lastCommand, 30)}"` : ''); this.logger.log(message, data); const response = await this.showPopupMessage(message, 'warn', false, [stopDebuggerAction]); @@ -895,12 +938,12 @@ export class BrightScriptDebugSession extends LoggingDebugSession { if (!error) { if (this.rokuAdapter.connected) { this.logger.info('Host connection was established before the main public process was completed'); - this.logger.log(`deployed to Roku@${this.launchConfiguration.host}`); + this.logger.log(`deployed to Roku@${this.deviceLabel}`); } else { this.logger.info('Main public process was completed but we are still waiting for a connection to the host'); this.rokuAdapter.on('connected', (status) => { if (status) { - this.logger.log(`deployed to Roku@${this.launchConfiguration.host}`); + this.logger.log(`deployed to Roku@${this.deviceLabel}`); } }); } @@ -915,7 +958,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { //if we are at a breakpoint, continue await this.rokuAdapter.continue(); //kill the app on the roku - // await this.rokuDeploy.pressHomeButton(this.launchConfiguration.host, this.launchConfiguration.remotePort); + // await this.rokuDeploy.keyPress({ device: this.launchConfiguration.device, key: 'Home', ecpPort: this.launchConfiguration.remotePort }); //convert a hostname to an ip address const deepLinkUrl = await util.resolveUrl(this.launchConfiguration.deepLinkUrl); //send the deep link http request @@ -946,7 +989,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { // Initialize PerfettoManager this.perfettoManager = new PerfettoManager({ - host: this.launchConfiguration.host, + device: this.launchConfiguration.device, rootDir: this.launchConfiguration.rootDir, remotePort: this.launchConfiguration.remotePort, ...this.launchConfiguration.profiling?.tracing @@ -1116,9 +1159,12 @@ export class BrightScriptDebugSession extends LoggingDebugSession { //delete any currently installed dev channel (if enabled to do so) try { if (this.launchConfiguration.deleteDevChannelBeforeInstall === true) { - await this.rokuDeploy.deleteInstalledChannel({ - ...this.launchConfiguration - } as any as RokuDeployOptions); + await this.rokuDeploy.deleteDevChannel({ + device: this.launchConfiguration.device, + password: this.launchConfiguration.password, + username: this.launchConfiguration.username, + packagePort: this.launchConfiguration.packagePort + }); } } catch (e) { const statusCode = e?.results?.response?.statusCode; @@ -1131,27 +1177,30 @@ export class BrightScriptDebugSession extends LoggingDebugSession { } const isConnected = this.rokuAdapter.once('app-ready'); - const options: RokuDeployOptions = { - ...this.launchConfiguration, - //typing fix - logLevel: LogLevelPriority[this.logger.logLevel], + const options: SideloadOptions = { + device: this.launchConfiguration.device, + password: this.launchConfiguration.password, + username: this.launchConfiguration.username, + packagePort: this.launchConfiguration.packagePort, + ecpPort: this.launchConfiguration.remotePort, + //sideload the zip that was already built from the staging folder (or supplied via packagePath) + zip: this.launchConfiguration.packagePath ?? util.getOutputZipPath({ outDir: this.launchConfiguration.outDir }), // enable the debug protocol if true remoteDebug: this.enableDebugProtocol, //necessary for capturing compile errors from the protocol (has no effect on telnet) remoteDebugConnectEarly: false, //we don't want to fail if there were compile errors...we'll let our compile error processor handle that failOnCompileError: true, + //deleting the dev channel (when enabled) was already handled at the start of this function + deleteDevChannel: false, + //the device was already sent to the home screen during configurationDone + close: false, //pass any upload form overrides the client may have configured packageUploadOverrides: this.launchConfiguration.packageUploadOverrides }; - //if packagePath is specified, use that info instead of outDir and outFile - if (this.launchConfiguration.packagePath) { - options.outDir = path.dirname(this.launchConfiguration.packagePath); - options.outFile = path.basename(this.launchConfiguration.packagePath); - } //publish the package to the target Roku - const publishPromise = this.rokuDeploy.publish(options).then(() => { + const publishPromise = this.rokuDeploy.sideload(options).then(() => { packageIsPublished = true; }).catch(async (e) => { const statusCode = e?.results?.response?.statusCode; @@ -1329,7 +1378,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { private async runAutomaticSceneGraphCommands(commands: string[]) { if (commands) { - let connection = new SceneGraphDebugCommandController(this.launchConfiguration.host, this.launchConfiguration.sceneGraphDebugCommandsPort); + let connection = new SceneGraphDebugCommandController(this.launchConfiguration.device, this.launchConfiguration.sceneGraphDebugCommandsPort); try { await connection.connect(); @@ -1428,17 +1477,9 @@ export class BrightScriptDebugSession extends LoggingDebugSession { util.log(`Executing task '${this.launchConfiguration.packageTask}' to assemble the app`); await this.sendCustomRequest('executeTask', { task: this.launchConfiguration.packageTask }); - const options = { - ...this.launchConfiguration - } as any as RokuDeployOptions; - //if packagePath is specified, use that info instead of outDir and outFile - if (this.launchConfiguration.packagePath) { - options.outDir = path.dirname(this.launchConfiguration.packagePath); - options.outFile = path.basename(this.launchConfiguration.packagePath); - } - const packagePath = this.launchConfiguration.packagePath ?? rokuDeploy.getOutputZipFilePath(options); + const packagePath = this.launchConfiguration.packagePath ?? util.getOutputZipPath({ outDir: this.launchConfiguration.outDir }); - if (!fsExtra.pathExistsSync(packagePath as string)) { + if (!fsExtra.pathExistsSync(packagePath)) { return this.shutdown(`Cancelling debug session. Package does not exist at '${packagePath}'`); } } else { @@ -1584,26 +1625,24 @@ export class BrightScriptDebugSession extends LoggingDebugSession { await this.sendCustomRequest('executeTask', { task: componentLibraries[i].packageTask }); } - const options: RokuDeployOptions = { - host: this.launchConfiguration.host, + const options: SideloadOptions = { + device: this.launchConfiguration.device, password: this.launchConfiguration.password, username: this.launchConfiguration.username || 'rokudev', - logLevel: LogLevelPriority[this.logger.logLevel], + packagePort: this.launchConfiguration.packagePort, + ecpPort: this.launchConfiguration.remotePort, + zip: componentLibraries[i].packagePath ?? util.getOutputZipPath({ outDir: compLibProject.outDir, outFile: compLibProject.outFile }), failOnCompileError: true, - outDir: compLibProject.outDir, - outFile: compLibProject.outFile, appType: 'dcl', + //installing a component library should never close or delete the sideloaded channel + close: false, + deleteDevChannel: false, packageUploadOverrides: componentLibraries[i].packageUploadOverrides || {} }; - if (componentLibraries[i].packagePath) { - options.outDir = path.dirname(componentLibraries[i].packagePath); - options.outFile = path.basename(componentLibraries[i].packagePath); - } - util.log(`Installing component library ${i} (${compLibProject.outFile})`); try { - await rokuDeploy.publish(options); + await rokuDeploy.sideload(options); util.log(`Installed component library ${i} (${compLibProject.outFile})`); } catch (error) { //do NOT continue installing further libraries (or publishing the main app) - a failed install @@ -1639,7 +1678,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { */ private async deleteAllComponentLibraries() { const deviceOptions = { - host: this.launchConfiguration.host, + device: this.launchConfiguration.device, password: this.launchConfiguration.password, username: this.launchConfiguration.username || 'rokudev' }; @@ -1725,11 +1764,11 @@ export class BrightScriptDebugSession extends LoggingDebugSession { * Did the component-library delete actually succeed, even though the device also reported a compile error? * When we delete a complib that a dependent still references, the device reports BOTH a compile error AND a * `Delete Succeeded` message - meaning the complib really was removed. roku-deploy attaches the parsed device - * messages (`{ errors, infos, successes }`) to the thrown error as `.results`; we look for the success there. + * messages (`{ errors, infos, successes }`) to the thrown error as `rokuMessages`; we look for the success there. * Absence of that success message means the delete did NOT happen, so it should be treated as a failure/retry. */ private wasComponentLibraryDeleteSuccessful(error: any): boolean { - const successes: string[] = error?.results?.successes ?? []; + const successes: string[] = error?.rokuMessages?.successes ?? []; return successes.some(message => /delete succeeded/i.test(message)); } @@ -2333,7 +2372,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { } else if (v.type === '$$Registry') { // This is a special scope variable used to load registry data via an ECP call // Send the registry ECP call for the `dev` app as side loaded apps are always `dev` - await populateVariableFromRegistryEcp({ host: this.launchConfiguration.host, remotePort: this.launchConfiguration.remotePort, appId: 'dev' }, v, this.variables, this.getEvaluateRefId.bind(this)); + await populateVariableFromRegistryEcp({ remotePort: this.launchConfiguration.remotePort, device: this.launchConfiguration.device, appId: 'dev' }, v, this.variables, this.getEvaluateRefId.bind(this)); } } catch (error) { logger.error(`Error getting variables for scope ${v.type}`, error); @@ -3062,14 +3101,14 @@ export class BrightScriptDebugSession extends LoggingDebugSession { */ protected async disconnectRequest(response: DebugProtocol.DisconnectResponse, args: DebugProtocol.DisconnectArguments, request?: DebugProtocol.Request) { //return to the home screen — best effort. The device may already be powered off or unreachable - //at disconnect time; without a guard pressHomeButton rejects (EHOSTDOWN / ECONNREFUSED / etc) + //at disconnect time; without a guard the home key press rejects (EHOSTDOWN / ECONNREFUSED / etc) //and because @vscode/debugadapter dispatches this method without awaiting the returned Promise, //that rejection escapes as an unhandledRejection and crashes the DAP process. //See https://github.com/rokucommunity/vscode-brightscript-language/issues/807 // https://github.com/rokucommunity/roku-debug/issues/332 if (!this.enableDebugProtocol) { try { - await this.rokuDeploy.pressHomeButton(this.launchConfiguration.host, this.launchConfiguration.remotePort); + await this.rokuDeploy.keyPress({ device: this.launchConfiguration.device, key: 'Home', ecpPort: this.launchConfiguration.remotePort }); } catch (e) { this.logger.warn('Failed to press home button during disconnect; device may be unreachable', e); } @@ -3112,8 +3151,8 @@ export class BrightScriptDebugSession extends LoggingDebugSession { try { let appStateResult = await rokuECP.getAppState({ - host: this.launchConfiguration.host, remotePort: this.launchConfiguration.remotePort, + device: this.launchConfiguration.device, appId: 'dev', requestOptions: { timeout: 300 } }); @@ -3126,8 +3165,8 @@ export class BrightScriptDebugSession extends LoggingDebugSession { // If the app supports Instant Resume and is running in the background or the app does not support Instant Resume and is running, sending this command terminates the app. // This means that we might need to send this command twice to terminate the app. await rokuECP.exitApp({ - host: this.launchConfiguration.host, remotePort: this.launchConfiguration.remotePort, + device: this.launchConfiguration.device, appId: 'dev', requestOptions: { timeout: 300 } }); @@ -3456,9 +3495,12 @@ export class BrightScriptDebugSession extends LoggingDebugSession { } else { const lastId = this.launchProgressId; this.sendEvent(new ProgressUpdateEvent(lastId, message)); - setTimeout(() => { + const endTimer = setTimeout(() => this.flushLaunchProgressEnd?.(), 1000); // add a slight delay before ending the progress to improve UX + this.flushLaunchProgressEnd = () => { + clearTimeout(endTimer); + this.flushLaunchProgressEnd = undefined; this.sendEvent(new ProgressEndEvent(lastId, message)); - }, 1000); // add a slight delay before ending the progress to improve UX + }; this.launchProgressId = undefined; } } @@ -3547,7 +3589,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { public async shutdown(errorMessage?: string, modal = false): Promise { if (this.shutdownPromise === undefined) { this.logger.log('[shutdown] Beginning shutdown sequence', errorMessage); - //Backstop: if the graceful shutdown hangs (e.g. pressHomeButton against an unreachable + //Backstop: if the graceful shutdown hangs (e.g. a home key press against an unreachable //device), force-exit anyway so we never leave an orphaned adapter running forever const forceExitTimer = setTimeout(() => { this.logger.error('[shutdown] graceful shutdown timed out; forcing exit'); @@ -3566,6 +3608,10 @@ export class BrightScriptDebugSession extends LoggingDebugSession { private async _shutdown(errorMessage?: string, modal = false): Promise { // Ensure any active launch progress bar is dismissed before showing error messages or the terminated event. this.sendLaunchProgress('end', 'Complete'); + // 'end' defers its ProgressEndEvent for UX; deliver it right now (whether from the line above or from an + // earlier 'end' whose delay has not elapsed yet) - the adapter exits before a pending timer would fire, + // which would leave the client's progress notification stuck open + this.flushLaunchProgressEnd?.(); //send the message FIRST before anything else. This improves the chances that the message will be displayed to the user try { @@ -3598,7 +3644,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { //press the home button to return to the home screen try { this.logger.log('Press home button'); - await this.rokuDeploy.pressHomeButton(this.launchConfiguration.host, this.launchConfiguration.remotePort); + await this.rokuDeploy.keyPress({ device: this.launchConfiguration.device, key: 'Home', ecpPort: this.launchConfiguration.remotePort }); } catch (e) { this.logger.error(e); } diff --git a/src/debugSession/Events.spec.ts b/src/debugSession/Events.spec.ts index 66e32c18..923dfecf 100644 --- a/src/debugSession/Events.spec.ts +++ b/src/debugSession/Events.spec.ts @@ -21,4 +21,28 @@ describe('Events', () => { expect(isLaunchStartEvent(null)).to.be.false; expect(isChannelPublishedEvent(null)).to.be.false; }); + + it('scrubs the rceToken from the launch config echoed in LaunchStartEvent and ChannelPublishedEvent', () => { + const launchConfiguration = { + rootDir: '/some/project', + device: { instanceUrl: 'https://device.rce.roku.com/instance/abc', rceToken: 'secret-token' } + } as any; + + const launchStartEvent = new LaunchStartEvent(launchConfiguration); + expect((launchStartEvent.body.device as any).rceToken).to.be.undefined; + expect((launchStartEvent.body.device as any).instanceUrl).to.equal('https://device.rce.roku.com/instance/abc'); + expect(launchStartEvent.body.rootDir).to.equal('/some/project'); + + const channelPublishedEvent = new ChannelPublishedEvent(launchConfiguration); + expect((channelPublishedEvent.body.launchConfiguration.device as any).rceToken).to.be.undefined; + + //the original config is left untouched (the debugger still needs the token) + expect(launchConfiguration.device.rceToken).to.equal('secret-token'); + }); + + it('leaves a local device config untouched in echoed events', () => { + const launchConfiguration = { device: { host: '1.2.3.4' } } as any; + const launchStartEvent = new LaunchStartEvent(launchConfiguration); + expect(launchStartEvent.body.device).to.eql({ host: '1.2.3.4' }); + }); }); diff --git a/src/debugSession/Events.ts b/src/debugSession/Events.ts index 0064192e..ee3ba6ba 100644 --- a/src/debugSession/Events.ts +++ b/src/debugSession/Events.ts @@ -1,7 +1,8 @@ /* eslint-disable @typescript-eslint/no-useless-constructor */ import type { DebugProtocol } from '@vscode/debugprotocol'; import type { BSDebugDiagnostic } from '../CompileErrorProcessor'; -import type { LaunchConfiguration } from '../LaunchConfiguration'; +import type { ResolvedLaunchConfiguration } from '../LaunchConfiguration'; +import { isRceDeviceConfig } from 'roku-deploy'; import type { ChanperfData } from '../ChanperfTracker'; import type { RendezvousHistory } from '../RendezvousTracker'; import type { ProjectStagingInfo } from '../managers/ProjectManager'; @@ -108,13 +109,26 @@ export function isChanperfEvent(event: any): event is ChanperfEvent { } +/** + * Copy a launch configuration for echoing back to the client, without the credentials that ride the + * device config: the rceToken is hydrated onto the device at normalize time (from the ROKU_RCE_TOKEN + * env var the extension injects) and must not travel back over DAP, where it would land in protocol + * logs and trace files. + */ +function scrubLaunchConfiguration(launchConfiguration: ResolvedLaunchConfiguration): ResolvedLaunchConfiguration { + if (typeof launchConfiguration?.device === 'object' && isRceDeviceConfig(launchConfiguration.device) && launchConfiguration.device.rceToken) { + return { ...launchConfiguration, device: { ...launchConfiguration.device, rceToken: undefined } }; + } + return launchConfiguration; +} + /** * Emitted when the launch sequence first starts. This is right after the debug session receives the `launch` request, * which happens before any zipping, sideloading, etc. */ -export class LaunchStartEvent extends CustomEvent { - constructor(launchConfiguration: LaunchConfiguration) { - super(launchConfiguration); +export class LaunchStartEvent extends CustomEvent { + constructor(launchConfiguration: ResolvedLaunchConfiguration) { + super(scrubLaunchConfiguration(launchConfiguration)); } } @@ -128,11 +142,11 @@ export function isLaunchStartEvent(event: any): event is LaunchStartEvent { /** * Emitted once the channel has been sideloaded to the channel and the session is ready to start actually debugging. */ -export class ChannelPublishedEvent extends CustomEvent<{ launchConfiguration: LaunchConfiguration }> { +export class ChannelPublishedEvent extends CustomEvent<{ launchConfiguration: ResolvedLaunchConfiguration }> { constructor( - launchConfiguration: LaunchConfiguration + launchConfiguration: ResolvedLaunchConfiguration ) { - super({ launchConfiguration }); + super({ launchConfiguration: scrubLaunchConfiguration(launchConfiguration) }); } } diff --git a/src/debugSession/ecpRegistryUtils.spec.ts b/src/debugSession/ecpRegistryUtils.spec.ts index bea67ed4..689b8f4f 100644 --- a/src/debugSession/ecpRegistryUtils.spec.ts +++ b/src/debugSession/ecpRegistryUtils.spec.ts @@ -1,10 +1,9 @@ import { expect } from 'chai'; -import type { Response } from 'request'; import { VariableType } from '../debugProtocol/events/responses/VariablesResponse'; import type { AugmentedVariable } from './BrightScriptDebugSession'; import { BrightScriptDebugSession } from './BrightScriptDebugSession'; import { populateVariableFromRegistryEcp } from './ecpRegistryUtils'; -import { rokuECP } from '../RokuECP'; +import { rokuDeploy } from 'roku-deploy'; import { createSandbox } from 'sinon'; const sinon = createSandbox(); @@ -44,17 +43,11 @@ describe('ecpRegistryUtils', () => { childVariables: [] }; - sinon.stub(rokuECP as any, 'doRequest').returns(Promise.resolve({ - body: ` - - OK - Plugin dev not found - - `, - statusCode: 200 - } as Response)); + sinon.stub(rokuDeploy, 'queryRegistry').resolves({ + sections: {} + }); - await populateVariableFromRegistryEcp({ host: '', appId: '' }, v, session['variables'], refFactory); + await populateVariableFromRegistryEcp({ device: { host: '' }, appId: '' }, v, session['variables'], refFactory); expect(v.childVariables.length).to.eql(1); expect(v.childVariables[0]).to.eql({ name: 'sections', @@ -99,23 +92,14 @@ describe('ecpRegistryUtils', () => { childVariables: [] }; - sinon.stub(rokuECP as any, 'doRequest').returns(Promise.resolve({ - body: ` - - - - 12345 - 12,34,dev - 28075 - - - OK - - `, - statusCode: 200 - } as Response)); + sinon.stub(rokuDeploy, 'queryRegistry').resolves({ + devId: '12345', + plugins: ['12', '34', 'dev'], + spaceAvailable: '28075', + sections: {} + }); - await populateVariableFromRegistryEcp({ host: '', appId: '' }, v, session['variables'], refFactory); + await populateVariableFromRegistryEcp({ device: { host: '' }, appId: '' }, v, session['variables'], refFactory); expect(v.childVariables.length).to.eql(4); expect(v.childVariables[0]).to.eql({ name: 'devId', @@ -191,46 +175,22 @@ describe('ecpRegistryUtils', () => { childVariables: [] }; - sinon.stub(rokuECP as any, 'doRequest').returns(Promise.resolve({ - body: ` - - - - 12345 - dev - 32590 - -
- section One - - - first key in section one - value one section one - - -
-
- section Two - - - first key in section two - value one section two - - - second key in section two - value two section two - - -
- - - OK - - `, - statusCode: 200 - } as Response)); + sinon.stub(rokuDeploy, 'queryRegistry').resolves({ + devId: '12345', + plugins: ['dev'], + spaceAvailable: '32590', + sections: { + 'section One': { + 'first key in section one': 'value one section one' + }, + 'section Two': { + 'first key in section two': 'value one section two', + 'second key in section two': 'value two section two' + } + } + }); - await populateVariableFromRegistryEcp({ host: '', appId: '' }, v, session['variables'], refFactory); + await populateVariableFromRegistryEcp({ device: { host: '' }, appId: '' }, v, session['variables'], refFactory); expect(v.childVariables.length).to.eql(4); expect(v.childVariables[0]).to.eql({ name: 'devId', @@ -355,21 +315,13 @@ describe('ecpRegistryUtils', () => { childVariables: [] }; - sinon.stub(rokuECP as any, 'doRequest').returns(Promise.resolve({ - body: ` - - FAILED - Plugin dev not found - - `, - statusCode: 200 - } as Response)); + sinon.stub(rokuDeploy, 'queryRegistry').rejects(new Error('Could not retrieve registry: Plugin dev not found')); - await populateVariableFromRegistryEcp({ host: '', appId: '' }, v, session['variables'], refFactory); + await populateVariableFromRegistryEcp({ device: { host: '' }, appId: '' }, v, session['variables'], refFactory); expect(v.childVariables.length).to.eql(1); expect(v.childVariables[0]).to.eql({ name: 'error', - value: `❌ Error: Plugin dev not found`, + value: `❌ Error: Could not retrieve registry: Plugin dev not found`, variablesReference: 0, type: VariableType.String, childVariables: [] @@ -385,21 +337,13 @@ describe('ecpRegistryUtils', () => { childVariables: [] }; - sinon.stub(rokuECP as any, 'doRequest').returns(Promise.resolve({ - body: ` - - FAILED - Device not keyed - - `, - statusCode: 200 - } as Response)); + sinon.stub(rokuDeploy, 'queryRegistry').rejects(new Error('Could not retrieve registry: Device not keyed')); - await populateVariableFromRegistryEcp({ host: '', appId: '' }, v, session['variables'], refFactory); + await populateVariableFromRegistryEcp({ device: { host: '' }, appId: '' }, v, session['variables'], refFactory); expect(v.childVariables.length).to.eql(1); expect(v.childVariables[0]).to.eql({ name: 'error', - value: `❌ Error: Device not keyed`, + value: `❌ Error: Could not retrieve registry: Device not keyed`, variablesReference: 0, type: VariableType.String, childVariables: [] @@ -415,20 +359,13 @@ describe('ecpRegistryUtils', () => { childVariables: [] }; - sinon.stub(rokuECP as any, 'doRequest').returns(Promise.resolve({ - body: ` - - FAILED - - `, - statusCode: 200 - } as Response)); + sinon.stub(rokuDeploy, 'queryRegistry').rejects(new Error('Could not retrieve registry: Unknown error')); - await populateVariableFromRegistryEcp({ host: '', appId: '' }, v, session['variables'], refFactory); + await populateVariableFromRegistryEcp({ device: { host: '' }, appId: '' }, v, session['variables'], refFactory); expect(v.childVariables.length).to.eql(1); expect(v.childVariables[0]).to.eql({ name: 'error', - value: `❌ Error: Unknown error`, + value: `❌ Error: Could not retrieve registry: Unknown error`, variablesReference: 0, type: VariableType.String, childVariables: [] @@ -444,16 +381,16 @@ describe('ecpRegistryUtils', () => { childVariables: [] }; - sinon.stub(rokuECP as any, 'doRequest').returns(Promise.resolve({ - body: `ECP command not allowed in Limited mode.`, - statusCode: 403 - } as Response)); + //roku-deploy throws an UnparsableDeviceResponseError carrying the device's + //plain-text explanation when the response body is not xml (a limited-mode refusal, + //for example), so that text still reaches the variables pane + sinon.stub(rokuDeploy, 'queryRegistry').rejects(new Error('Could not retrieve registry: ECP command not allowed in Limited mode.')); - await populateVariableFromRegistryEcp({ host: '', appId: '' }, v, session['variables'], refFactory); + await populateVariableFromRegistryEcp({ device: { host: '' }, appId: '' }, v, session['variables'], refFactory); expect(v.childVariables.length).to.eql(1); expect(v.childVariables[0]).to.eql({ name: 'error', - value: `❌ Error: ECP command not allowed in Limited mode.`, + value: `❌ Error: Could not retrieve registry: ECP command not allowed in Limited mode.`, variablesReference: 0, type: VariableType.String, childVariables: [] diff --git a/src/interfaces.ts b/src/interfaces.ts index 21154ff4..622087e5 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -1,3 +1,5 @@ +import type { DeviceConfig } from 'roku-deploy'; + export enum HighLevelType { primative = 'primative', array = 'array', @@ -16,7 +18,13 @@ export interface RokuAdapterEvaluateResponse { } export interface AdapterOptions { - host: string; + /** + * The roku-deploy device config for the target device. The debug session normalizes whatever + * addressing the launch config supplied (including the deprecated `host` field) into a concrete + * device config before constructing an adapter, so this is the only way adapters address the + * device. + */ + device: DeviceConfig; brightScriptConsolePort?: number; remotePort?: number; /** diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 09ec64ea..77d504c0 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -2198,12 +2198,11 @@ describe('ComponentLibraryProject', () => { it('computes stagingDir before calling getFileMappings', async () => { delete params.stagingDir; let project = new ComponentLibraryProject(params); - // The default stagingDir is resolved at construction time by roku-deploy - let defaultStagingDir = project.stagingDir; + //roku-deploy returns staging-relative dest paths; getFileMappings makes them absolute sinon.stub(rokuDeploy, 'getFilePaths').returns(Promise.resolve([ - { src: s`${rootDir}/manifest`, dest: s`${defaultStagingDir}/manifest` }, - { src: s`${rootDir}/source/main.brs`, dest: s`${defaultStagingDir}/source/main.brs` } + { src: s`${rootDir}/manifest`, dest: 'manifest' }, + { src: s`${rootDir}/source/main.brs`, dest: s`source/main.brs` } ])); sinon.stub(Project.prototype, 'stage').returns(Promise.resolve()); sinon.stub(util, 'convertManifestToObject').returns(Promise.resolve({})); diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index c4035e3a..ef3be12a 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -1,6 +1,6 @@ import * as fsExtra from 'fs-extra'; import * as path from 'path'; -import { rokuDeploy, RokuDeploy, util as rokuDeployUtil } from 'roku-deploy'; +import { rokuDeploy, util as rokuDeployUtil } from 'roku-deploy'; import type { FileEntry } from 'roku-deploy'; import * as fastGlob from 'fast-glob'; import type { BreakpointManager } from './BreakpointManager'; @@ -419,7 +419,7 @@ export class Project { throw new Error('outDir is required'); } this.outDir = fileUtils.standardizePath(params.outDir); - this.stagingDir = params.stagingDir ?? rokuDeploy.getOptions(this).stagingDir; + this.stagingDir = params.stagingDir ?? util.getStagingDir({ outDir: this.outDir }); this.bsConst = params.bsConst; this.sourceDirs = (params.sourceDirs ?? []) //standardize every sourcedir @@ -434,6 +434,11 @@ export class Project { } public rootDir: string; public outDir: string; + /** + * The filename of the zip package that gets created from the staging folder (relative to `outDir`). + * Component libraries override this with their computed out file name. + */ + public outFile = 'roku-deploy.zip'; public packagePath: string; public sourceDirs: string[]; public files: Array; @@ -483,13 +488,10 @@ export class Project { } //copy all project files to the staging folder - await rokuDeploy.prepublishToStaging({ + await rokuDeploy.stage({ rootDir: this.rootDir, - stagingDir: this.stagingDir, - files: this.fileMappings, - outDir: this.outDir, - //we already fetched the file mappings ourselves, so roku-deploy doesn't need to glob the files again - resolveFilesArray: false + files: this.files, + out: this.stagingDir }); await this.preprocessStagingFiles(); @@ -1039,34 +1041,33 @@ export class Project { * @param stagingPath */ public async zipPackage(params: { retainStagingFolder: boolean }) { - const options = rokuDeploy.getOptions({ - ...this, - ...params - }); - let packagePath = this.packagePath; if (!this.packagePath) { //make sure the output folder exists - await fsExtra.ensureDir(options.outDir); + await fsExtra.ensureDir(this.outDir); - packagePath = rokuDeploy.getOutputZipFilePath(options); + packagePath = util.getOutputZipPath({ outDir: this.outDir, outFile: this.outFile }); } //ensure the manifest file exists in the staging folder - if (!await rokuDeployUtil.fileExistsCaseInsensitive(`${options.stagingDir}/manifest`)) { - throw new Error(`Cannot zip package: missing manifest file in "${options.stagingDir}"`); + if (!await rokuDeployUtil.fileExistsCaseInsensitive(`${this.stagingDir}/manifest`)) { + throw new Error(`Cannot zip package: missing manifest file in "${this.stagingDir}"`); } // create a zip of the staging folder - await rokuDeploy.zipFolder(options.stagingDir, packagePath, undefined, [ - '**/*', - //exclude sourcemap files (they're large and can't be parsed on-device anyway...) - '!**/*.map' - ]); + await rokuDeploy.zip({ + dir: this.stagingDir, + out: packagePath, + files: [ + '**/*', + //exclude sourcemap files (they're large and can't be parsed on-device anyway...) + '!**/*.map' + ] + }); //delete the staging folder unless told to retain it. - if (options.retainStagingDir !== true) { - await fsExtra.remove(options.stagingDir); + if (params.retainStagingFolder !== true) { + await fsExtra.remove(this.stagingDir); } } @@ -1075,7 +1076,10 @@ export class Project { * (`dest` paths are relative in later versions of roku-deploy) */ protected async getFileMappings() { - let fileMappings = await rokuDeploy.getFilePaths(this.files, this.rootDir, true, this.stagingDir); + let fileMappings = await rokuDeploy.getFilePaths({ files: this.files, rootDir: this.rootDir }); + for (let fileMapping of fileMappings) { + fileMapping.dest = s`${this.stagingDir}/${fileMapping.dest}`; + } return fileMappings; } @@ -1099,7 +1103,6 @@ export class ComponentLibraryProject extends Project { this.install = params.install ?? false; this.enablePostfix = params.enablePostfix ?? true; } - public outFile: string; public libraryIndex: number; public install: boolean; /** diff --git a/src/util.spec.ts b/src/util.spec.ts index f867c0c8..dd5605d1 100644 --- a/src/util.spec.ts +++ b/src/util.spec.ts @@ -18,6 +18,60 @@ beforeEach(() => { describe('Util', () => { + describe('hydrateRceTokenFromEnv', () => { + let originalEnvToken: string | undefined; + + beforeEach(() => { + originalEnvToken = process.env.ROKU_RCE_TOKEN; + }); + + afterEach(() => { + if (originalEnvToken === undefined) { + delete process.env.ROKU_RCE_TOKEN; + } else { + process.env.ROKU_RCE_TOKEN = originalEnvToken; + } + }); + + it('hydrates the token onto a tokenless RCE device option', () => { + process.env.ROKU_RCE_TOKEN = 'env-token'; + expect(util.hydrateRceTokenFromEnv({ instanceUrl: 'https://device.rce.roku.com/instance/abc' })).to.eql({ + instanceUrl: 'https://device.rce.roku.com/instance/abc', + rceToken: 'env-token' + }); + }); + + it('a token already on the device option wins over the env var', () => { + process.env.ROKU_RCE_TOKEN = 'env-token'; + expect(util.hydrateRceTokenFromEnv({ id: 83, rceToken: 'config-token' })).to.eql({ + id: 83, + rceToken: 'config-token' + }); + }); + + it('returns a tokenless RCE device option unchanged when the env var is not set', () => { + delete process.env.ROKU_RCE_TOKEN; + const device = { esn: 'esn-value' }; + expect(util.hydrateRceTokenFromEnv(device)).to.equal(device); + }); + + it('leaves local device configs untouched', () => { + process.env.ROKU_RCE_TOKEN = 'env-token'; + const localDevice = { host: '1.2.3.4' }; + expect(util.hydrateRceTokenFromEnv(localDevice)).to.equal(localDevice); + }); + }); + + describe('getDeviceLabel', () => { + it('identifies each device addressing scheme without leaking credentials', () => { + expect(util.getDeviceLabel({ host: '1.2.3.4' })).to.equal('1.2.3.4'); + expect(util.getDeviceLabel({ instanceUrl: 'https://device.rce.roku.com/instance/abc', rceToken: 'secret' })).to.equal('https://device.rce.roku.com/instance/abc'); + expect(util.getDeviceLabel({ id: 83, rceToken: 'secret' })).to.equal('83'); + expect(util.getDeviceLabel({ esn: 'esn-value', rceToken: 'secret' })).to.equal('esn-value'); + expect(util.getDeviceLabel(undefined)).to.equal(undefined); + }); + }); + describe('hasNonNullishProperty', () => { it('detects objects with only nullish props or no props at all', () => { expect(util.hasNonNullishProperty({})).to.be.false; diff --git a/src/util.ts b/src/util.ts index 8180bdd1..369ab533 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,5 +1,6 @@ import * as fs from 'fs'; import * as fsExtra from 'fs-extra'; +import * as path from 'path'; import * as net from 'net'; import * as portfinder from 'portfinder'; import type { BrightScriptDebugSession } from './debugSession/BrightScriptDebugSession'; @@ -16,9 +17,33 @@ import { OutputEvent } from '@vscode/debugadapter'; import * as xml2js from 'xml2js'; import { isPromise } from 'util/types'; import type { Logger } from '@rokucommunity/logger'; +import type { DeviceConfig, RokuDeploySocket } from 'roku-deploy'; +import { isRceDeviceConfigById, isRceDeviceConfigByUrl, isRceDeviceConfig } from 'roku-deploy'; const request = r as typeof requestType; class Util { + /** + * Resolve a project's staging folder the same way roku-deploy's `stage()` does: the default + * staging folder name inside `outDir`. roku-deploy does not expose this resolution, so it is + * rolled here and must stay in step with roku-deploy's own defaults. + */ + public getStagingDir(options: { outDir?: string; cwd?: string }): string { + return path.resolve(options.cwd ?? process.cwd(), options.outDir ?? './out', '.roku-deploy-staging'); + } + + /** + * Resolve a project's output zip path the same way roku-deploy's `zip()` does: `outFile` + * inside `outDir`, with a `.zip` extension enforced. roku-deploy does not expose this + * resolution, so it is rolled here and must stay in step with roku-deploy's own defaults. + */ + public getOutputZipPath(options: { outDir?: string; outFile?: string; cwd?: string }): string { + let out = path.resolve(options.cwd ?? process.cwd(), options.outDir ?? './out', options.outFile ?? 'roku-deploy.zip'); + if (!out.toLowerCase().endsWith('.zip')) { + out += '.zip'; + } + return out; + } + /** * If the path does not have a trailing slash, one is appended to it * @param dirPath @@ -554,13 +579,50 @@ class Util { }); } + /** + * Hydrate the Cloud Emulator api token onto an RCE device option from the `ROKU_RCE_TOKEN` + * environment variable when the option does not already carry one. The VS Code extension + * injects that variable into the debug adapter process so the token does not have to travel + * through the launch config (where it would end up in DAP traffic and logs); a token supplied + * directly on the device option always wins. Anything that is not a tokenless RCE device + * config is returned unchanged. + */ + public hydrateRceTokenFromEnv(device: T): T { + if (device && isRceDeviceConfig(device) && !device.rceToken && process.env.ROKU_RCE_TOKEN) { + return { ...device, rceToken: process.env.ROKU_RCE_TOKEN } as T; + } + return device; + } + + /** + * A short human-readable identifier for a device, safe for log and error messages (never + * includes credentials like the rceToken). A local device is identified by its host and an RCE + * device by its instanceUrl, id, or esn. + */ + public getDeviceLabel(device: DeviceConfig): string { + //a device may legitimately be absent on early error paths (before a session is configured) + if (!device) { + return undefined; + } + if (isRceDeviceConfig(device)) { + if (isRceDeviceConfigByUrl(device)) { + return device.instanceUrl; + } + return isRceDeviceConfigById(device) ? String(device.id) : device.esn; + } + return device.host; + } + /** * Register the socket events for logging - * @param socket - the socket to listen to for events + * @param socket - the socket to listen to for events. Accepts a real `net.Socket` as well as + * roku-deploy's `RokuDeploySocket` (an RCE device's telnet socket is not a real tcp socket, so its + * address-related fields are always undefined; the events below that never fire for it are + * harmless no-ops) * @param logger - the logger to use for logging * @param socketType - the type of socket (e.g. "client", "server") */ - public registerSocketLogging(socket: net.Socket, logger: Logger, socketType: string) { + public registerSocketLogging(socket: net.Socket | RokuDeploySocket, logger: Logger, socketType: string) { // create a new child logger for the socket events let socketLogger = logger.createLogger(`[${socketType}]`); @@ -613,7 +675,7 @@ class Util { }); } - private getSocketAddressForLogs(socket: net.Socket, ip?: string, port?: number, family?: number): string { + private getSocketAddressForLogs(socket: net.Socket | RokuDeploySocket, ip?: string, port?: number, family?: number): string { let familyString: string; if (typeof family === 'number') { familyString = `IPv${family}`;