From 99ee35335d59824621c24e6447c5abe39bdc31be Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 2 Dec 2024 16:12:58 -0500 Subject: [PATCH 1/4] Fix issues detecting when a device needs to check for updates --- src/Errors.ts | 13 ++++++------- src/RokuDeploy.spec.ts | 16 ++++++++++++++++ src/RokuDeploy.ts | 24 ++++++++++++++---------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/Errors.ts b/src/Errors.ts index 7f480aa..22fb1d2 100644 --- a/src/Errors.ts +++ b/src/Errors.ts @@ -1,4 +1,4 @@ -import type { RokuMessages } from './RokuDeploy'; +import type { HttpResponse, RokuMessages } from './RokuDeploy'; export class InvalidDeviceResponseCodeError extends Error { constructor(message: string, public results?: any) { @@ -57,15 +57,14 @@ export class MissingRequiredOptionError extends Error { } export class UpdateCheckRequiredError extends Error { - results: any; - - cause: Error; - constructor(originalError: Error) { + constructor(response: HttpResponse) { super(); this.message = `Your device needs to check for updates before accepting connections. Please navigate to System Settings and check for updates and then try again.\n\nhttps://support.roku.com/article/208755668.`; - this.results = { response: { statusCode: 577 } }; - this.cause = originalError; + //this exact structure helps `roku-debug` detect this error by finding this status code and then showing a nice popup + this.results = { response: { ...response ?? {}, statusCode: 500 } }; Object.setPrototypeOf(this, UpdateCheckRequiredError.prototype); } + + results: any; } diff --git a/src/RokuDeploy.spec.ts b/src/RokuDeploy.spec.ts index dfe03fc..60ac326 100644 --- a/src/RokuDeploy.spec.ts +++ b/src/RokuDeploy.spec.ts @@ -3642,6 +3642,22 @@ https://support.roku.com/article/208755668.`)); }); }); + describe('isUpdateCheckRequiredResponse', () => { + it('matches on actual response from device', () => { + const response = `\n\n \n \n Roku Development Kit \n\n \n\n\n
\n\n
\n\n \n \n\n\n
\n\n Please make sure that your Roku device is connected to internet, and running most recent software version (d=953108)\n\n
\n\n\n\n`; + expect( + rokuDeploy['isUpdateCheckRequiredResponse'](response) + ).to.be.true; + }); + + it('matches with some variations to the message', () => { + const response = `" FAILED tocheck\tfor softwareupdate"`; + expect( + rokuDeploy['isUpdateCheckRequiredResponse'](response) + ).to.be.true; + }); + }); + function mockDoGetRequest(body = '', statusCode = 200) { return sinon.stub(rokuDeploy as any, 'doGetRequest').callsFake((params) => { let results = { response: { statusCode: statusCode }, body: body }; diff --git a/src/RokuDeploy.ts b/src/RokuDeploy.ts index 7c3ccdd..9996f6d 100644 --- a/src/RokuDeploy.ts +++ b/src/RokuDeploy.ts @@ -468,19 +468,16 @@ export class RokuDeploy { if (this.isCompileError(replaceError.message) && options.failOnCompileError) { throw new errors.CompileError('Compile error', replaceError, replaceError.results); } else { - try { - response = await this.doPostRequest(requestOptions); - } catch (installError: any) { - switch (installError.results.response.statusCode) { - case 577: - throw new errors.UpdateCheckRequiredError(installError); - default: - throw installError; - } - } + requestOptions.formData.mysubmit = 'Install'; + response = await this.doPostRequest(requestOptions); } } + //if we got a non-error status code, but the body includes a message about needing to update, throw a special error + if (this.isUpdateCheckRequiredResponse(response.body)) { + throw new errors.UpdateCheckRequiredError(response); + } + if (options.failOnCompileError) { if (this.isCompileError(response.body)) { throw new errors.CompileError('Compile error', response, this.getRokuMessagesFromResponseBody(response.body)); @@ -512,6 +509,13 @@ export class RokuDeploy { return !!/install\sfailure:\scompilation\sfailed/i.exec(responseHtml); } + /** + * Does the response look like a compile error + */ + private isUpdateCheckRequiredResponse(responseHtml: string) { + return !!/["']\s*Failed\s*to\s*check\s*for\s*software\s*update\s*["']/i.exec(responseHtml); + } + /** * Converts existing loaded package to squashfs for faster loading packages * @param options From 885502bb0c6235845c7e1c30dfc2795361d0599d Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 2 Dec 2024 16:16:38 -0500 Subject: [PATCH 2/4] Fix failing test --- src/Errors.ts | 4 +++- src/RokuDeploy.spec.ts | 17 +++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Errors.ts b/src/Errors.ts index 22fb1d2..6f9009f 100644 --- a/src/Errors.ts +++ b/src/Errors.ts @@ -58,9 +58,11 @@ export class MissingRequiredOptionError extends Error { export class UpdateCheckRequiredError extends Error { + static MESSAGE = `Your device needs to check for updates before accepting connections. Please navigate to System Settings and check for updates and then try again.\n\nhttps://support.roku.com/article/208755668.`; + constructor(response: HttpResponse) { super(); - this.message = `Your device needs to check for updates before accepting connections. Please navigate to System Settings and check for updates and then try again.\n\nhttps://support.roku.com/article/208755668.`; + this.message = UpdateCheckRequiredError.MESSAGE; //this exact structure helps `roku-debug` detect this error by finding this status code and then showing a nice popup this.results = { response: { ...response ?? {}, statusCode: 500 } }; Object.setPrototypeOf(this, UpdateCheckRequiredError.prototype); diff --git a/src/RokuDeploy.spec.ts b/src/RokuDeploy.spec.ts index 60ac326..707a8b4 100644 --- a/src/RokuDeploy.spec.ts +++ b/src/RokuDeploy.spec.ts @@ -1103,17 +1103,18 @@ describe('index', () => { }); }); - it('rejects when response contains invalid password status code', () => { + it('rejects when response contains invalid password status code', async () => { options.failOnCompileError = true; - mockDoPostRequest('', 577); + mockDoPostRequest(`'Failed to check for software update'`, 200); - return rokuDeploy.publish(options).then(() => { + try { + await rokuDeploy.publish(options); assert.fail('Should not have succeeded due to roku server compilation failure'); - }, (err) => { - expect(err.message).to.be.a('string').and.satisfy(msg => msg.startsWith(`Your device needs to check for updates before accepting connections. Please navigate to System Settings and check for updates and then try again. - -https://support.roku.com/article/208755668.`)); - }); + } catch (err) { + expect((err as any).message).to.eql( + errors.UpdateCheckRequiredError.MESSAGE + ); + } }); it('handles successful deploy', () => { From b9d8c1db3625dc8d4b5bfd99dc7075f17f422a51 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 3 Dec 2024 12:01:01 -0500 Subject: [PATCH 3/4] Improved error types --- package.json | 2 +- src/Errors.ts | 37 +++++++++++++++++++++++++++++++++---- src/RokuDeploy.ts | 29 +++++++++++++++++++++-------- 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 2567086..55d6fec 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "publish-coverage": "nyc report --reporter=text-lcov | coveralls" }, "dependencies": { + "@types/request": "^2.47.0", "chalk": "^2.4.2", "dateformat": "^3.0.3", "dayjs": "^1.11.0", @@ -42,7 +43,6 @@ "@types/mocha": "^9.0.0", "@types/node": "^16.11.3", "@types/q": "^1.5.8", - "@types/request": "^2.47.0", "@types/sinon": "^10.0.4", "@types/xml2js": "^0.4.5", "@typescript-eslint/eslint-plugin": "5.1.0", diff --git a/src/Errors.ts b/src/Errors.ts index 6f9009f..1ca78cc 100644 --- a/src/Errors.ts +++ b/src/Errors.ts @@ -1,4 +1,5 @@ import type { HttpResponse, RokuMessages } from './RokuDeploy'; +import type * as requestType from 'request'; export class InvalidDeviceResponseCodeError extends Error { constructor(message: string, public results?: any) { @@ -56,17 +57,45 @@ export class MissingRequiredOptionError extends Error { } } +/** + * This error is thrown when a Roku device refuses to accept connections because it requires the user to check for updates (even if no updates are actually available). + */ export class UpdateCheckRequiredError extends Error { static MESSAGE = `Your device needs to check for updates before accepting connections. Please navigate to System Settings and check for updates and then try again.\n\nhttps://support.roku.com/article/208755668.`; - constructor(response: HttpResponse) { + constructor( + public response: HttpResponse, + public requestOptions: requestType.OptionsWithUrl, + public cause?: Error + ) { super(); this.message = UpdateCheckRequiredError.MESSAGE; - //this exact structure helps `roku-debug` detect this error by finding this status code and then showing a nice popup - this.results = { response: { ...response ?? {}, statusCode: 500 } }; Object.setPrototypeOf(this, UpdateCheckRequiredError.prototype); } +} + +export function isUpdateCheckRequiredError(e: any): e is UpdateCheckRequiredError { + return e?.constructor?.name === 'UpdateCheckRequiredError'; +} + +/** + * This error is thrown when a Roku device ends the connection unexpectedly, causing an 'ECONNRESET' error. Typically this happens when the device needs to check for updates (even if no updates are available), but it can also happen for other reasons. + */ +export class ConnectionResetError extends Error { + + static MESSAGE = `The Roku device ended the connection unexpectedly and may need to check for updates before accepting connections. Please navigate to System Settings and check for updates and then try again.\n\nhttps://support.roku.com/article/208755668.`; + + constructor(error: Error, requestOptions: requestType.OptionsWithUrl) { + super(); + this.message = ConnectionResetError.MESSAGE; + this.cause = error; + Object.setPrototypeOf(this, ConnectionResetError.prototype); + } + + public cause?: Error; +} - results: any; +export function isConnectionResetError(e: any): e is ConnectionResetError { + return e?.constructor?.name === 'ConnectionResetError'; } diff --git a/src/RokuDeploy.ts b/src/RokuDeploy.ts index 9996f6d..e8b9d8f 100644 --- a/src/RokuDeploy.ts +++ b/src/RokuDeploy.ts @@ -462,20 +462,33 @@ export class RokuDeploy { //try to "replace" the channel first since that usually works. let response: HttpResponse; try { - response = await this.doPostRequest(requestOptions); - } catch (replaceError: any) { - //fail if this is a compile error - if (this.isCompileError(replaceError.message) && options.failOnCompileError) { - throw new errors.CompileError('Compile error', replaceError, replaceError.results); - } else { - requestOptions.formData.mysubmit = 'Install'; + try { response = await this.doPostRequest(requestOptions); + } catch (replaceError: any) { + //fail if this is a compile error + if (this.isCompileError(replaceError.message) && options.failOnCompileError) { + throw new errors.CompileError('Compile error', replaceError, replaceError.results); + } else { + requestOptions.formData.mysubmit = 'Install'; + response = await this.doPostRequest(requestOptions); + } + } + } catch (e: any) { + //if this is a 577 error, we have high confidence that the device needs to do an update check + if (e?.results?.response?.statusCode === 577) { + throw new errors.UpdateCheckRequiredError(response, requestOptions, e); + + //a reset connection could be cause by several things, but most likely it's due to the device needing to check for updates + } else if (e.code === 'ECONNRESET') { + throw new errors.ConnectionResetError(e, requestOptions); + } else { + throw e; } } //if we got a non-error status code, but the body includes a message about needing to update, throw a special error if (this.isUpdateCheckRequiredResponse(response.body)) { - throw new errors.UpdateCheckRequiredError(response); + throw new errors.UpdateCheckRequiredError(response, requestOptions); } if (options.failOnCompileError) { From 614e7a2cdeb2eeea84ec45edd25b3865498a418a Mon Sep 17 00:00:00 2001 From: Christian Holbrook Date: Tue, 3 Dec 2024 15:55:06 -0700 Subject: [PATCH 4/4] Add unit tests to get to 100% coverage --- src/RokuDeploy.spec.ts | 39 +++++++++++++++++++++++++++++++++++++++ src/RokuDeploy.ts | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/RokuDeploy.spec.ts b/src/RokuDeploy.spec.ts index 707a8b4..a05e50a 100644 --- a/src/RokuDeploy.spec.ts +++ b/src/RokuDeploy.spec.ts @@ -1207,6 +1207,45 @@ describe('index', () => { } assert.fail('Should not have succeeded'); }); + + it('Should throw an excpetion', async () => { + options.failOnCompileError = true; + mockDoPostRequest('', 577); + + try { + await rokuDeploy.publish(options); + } catch (e) { + assert.ok('Exception was thrown as expected'); + expect(e).to.be.instanceof(errors.UpdateCheckRequiredError); + return; + } + assert.fail('Should not have succeeded'); + }); + + class ErrorWithConnectionResetCode extends Error { + code; + + constructor(code = 'ECONNRESET') { + super(); + this.code = code; + } + } + + it('Should throw an excpetion', async () => { + options.failOnCompileError = true; + sinon.stub(rokuDeploy as any, 'doPostRequest').callsFake((params) => { + throw new ErrorWithConnectionResetCode(); + }); + + try { + await rokuDeploy.publish(options); + } catch (e) { + assert.ok('Exception was thrown as expected'); + expect(e).to.be.instanceof(errors.ConnectionResetError); + return; + } + assert.fail('Should not have succeeded'); + }); }); describe('convertToSquashfs', () => { diff --git a/src/RokuDeploy.ts b/src/RokuDeploy.ts index e8b9d8f..6a734dd 100644 --- a/src/RokuDeploy.ts +++ b/src/RokuDeploy.ts @@ -475,7 +475,7 @@ export class RokuDeploy { } } catch (e: any) { //if this is a 577 error, we have high confidence that the device needs to do an update check - if (e?.results?.response?.statusCode === 577) { + if (e.results?.response?.statusCode === 577) { throw new errors.UpdateCheckRequiredError(response, requestOptions, e); //a reset connection could be cause by several things, but most likely it's due to the device needing to check for updates