From 4fdf71bb045ad7b80523a5dc729c2f6244b7a793 Mon Sep 17 00:00:00 2001 From: Andrei Rusu Date: Sat, 25 Jul 2020 14:33:46 +0200 Subject: [PATCH] Fixed #2458 - correctly handling 5xx internal server errors from remote selenium/webdriver services. --- lib/api/element-commands/_waitFor.js | 22 +++-- lib/assertion/assertion-runner.js | 16 ++++ lib/http/response.js | 31 ++++--- lib/transport/jsonwire.js | 22 ++++- lib/transport/webdriver.js | 2 + test/lib/mockserver.js | 19 ++++- .../sampleTestWithServerError.js | 19 +++++ test/src/element/testElementCommands.js | 42 +++++++++ test/src/index/testNightwatchIndex.js | 5 +- test/src/runner/cli/testParallelExecution.js | 6 +- test/src/runner/testRunWithServerErrors.js | 85 +++++++++++++++++++ test/src/runner/testRunnerSessionCreate.js | 2 +- 12 files changed, 242 insertions(+), 29 deletions(-) create mode 100644 test/sampletests/withservererrors/sampleTestWithServerError.js create mode 100644 test/src/runner/testRunWithServerErrors.js diff --git a/lib/api/element-commands/_waitFor.js b/lib/api/element-commands/_waitFor.js index c935f96e5c..b1daeb2770 100644 --- a/lib/api/element-commands/_waitFor.js +++ b/lib/api/element-commands/_waitFor.js @@ -162,7 +162,9 @@ class WaitForElement extends ElementCommand { return this.assert({ response, passed: true, - err: {} + err: { + expected: this.expectedValue + } }); } @@ -173,7 +175,8 @@ class WaitForElement extends ElementCommand { response, passed: false, err: { - actual, expected + actual, + expected } }); } @@ -182,23 +185,30 @@ class WaitForElement extends ElementCommand { this.elapsedTime = this.executor.elapsedTime; const {reporter} = this.client; + const {result} = response; const {elapsedTime, message, abortOnFailure, stackTrace} = this; const runner = new AssertionRunner({ - passed, err, message, abortOnFailure, stackTrace, reporter, elapsedTime + passed, + err, + message, + abortOnFailure, + stackTrace, + reporter, + elapsedTime }); - return runner.run() + return runner.run(result) .catch(err => (err)) .then(err => { if (err instanceof Error) { err.abortOnFailure = this.abortOnFailure; - return this.complete(err, response.result); + return this.complete(err, result); } // FIXME // Due to backwards compatibility reasons, we keep here the simplified result object - return this.complete(null, response.result); + return this.complete(null, result); }); } diff --git a/lib/assertion/assertion-runner.js b/lib/assertion/assertion-runner.js index 55a0d87936..66aa9ed17d 100644 --- a/lib/assertion/assertion-runner.js +++ b/lib/assertion/assertion-runner.js @@ -1,6 +1,16 @@ const NightwatchAssertion = require('./assertion.js'); module.exports = class AssertionRunner { + static isServerError(result = {}) { + if (!result) { + return false; + } + + const {status, error, httpStatusCode} = result; + + return (status === -1 && error && httpStatusCode && httpStatusCode.toString().startsWith('5')); + } + /** * @param opts = {passed, err, calleeFn, message, stackTrace, abortOnFailure, reporter, addExpected = true} */ @@ -27,6 +37,12 @@ module.exports = class AssertionRunner { let assertResult; let isError = false; + if (commandResult && AssertionRunner.isServerError(commandResult)) { + const errorMessage = commandResult.error || 'Unknown server error'; + this.assertion.passed = false; + this.assertion.actual = `Server Error: ${errorMessage}`; + } + try { await this.assertion.assert(); reporter.registerPassed(this.assertion.message); diff --git a/lib/http/response.js b/lib/http/response.js index 2803a25c23..75a1af61e9 100644 --- a/lib/http/response.js +++ b/lib/http/response.js @@ -1,7 +1,7 @@ const EventEmitter = require('events'); const HttpUtil = require('./http.js'); const Formatter = require('./formatter.js'); -const {Logger} = require('../utils'); +const Utils = require('../utils'); class HttpResponse extends EventEmitter { @@ -71,26 +71,35 @@ class HttpResponse extends EventEmitter { }); } - isResponseJson(data) { + isResponseJson() { return this.res.headers['content-type'] && this.res.headers['content-type'].indexOf('application/json') > -1; } + isServerError() { + return this.res.statusCode.toString().startsWith('5') + } + parseResponseData(data) { let result; + data = data.toString(); data = Formatter.stripUnknownChars(data); - if (data && !this.isResponseJson(data)) { - return { - status: -1, - value: data - }; - } - try { result = JSON.parse(data); } catch (err) { - console.error(Logger.colors.red('Error processing the server response:'), '\n', data); - result = {status: -1, value: err.message}; + result = data; + } + + // sometimes the server non-json error responses end up parsed as strings and the error in undetected + if (Utils.isString(result) && this.isServerError()) { + return { + status: -1, + ['[[isNightwatch]]']: true, + value: { + error: 'internal server error', + message: result + } + }; } return result; diff --git a/lib/transport/jsonwire.js b/lib/transport/jsonwire.js index 2d918e6def..c262e48d61 100644 --- a/lib/transport/jsonwire.js +++ b/lib/transport/jsonwire.js @@ -83,6 +83,16 @@ class JsonWireProtocol extends Transport { } formatCommandResponseData(result) { + if (!result.value || typeof result.value != 'object') { + return; + } + + if (result['[[isNightwatch]]']) { + delete result['[[isNightwatch]]']; + + return; + } + if (result.class) { delete result.class; } @@ -91,10 +101,6 @@ class JsonWireProtocol extends Transport { delete result.hCode; } - if (!result.value || typeof result.value != 'object') { - return; - } - let errorDetails = null; if (result.value.screen) { @@ -156,6 +162,14 @@ class JsonWireProtocol extends Transport { errorMessage += ` ${response.statusCode} ${response.statusMessage}`; } + if (response && response.headers && (response.headers['content-type'] === 'text/html' || response.headers['content-type'] === 'text/plain')) { + errorMessage = errorMessage.replace(/(<([^>]+)>)/gi, ''); // strip html tags + } + + if (errorMessage.length > 50) { + errorMessage = errorMessage.substr(0, 50) + '...'; + } + return { status: -1, state: result.state || '', diff --git a/lib/transport/webdriver.js b/lib/transport/webdriver.js index c9f196f622..90e12990ac 100644 --- a/lib/transport/webdriver.js +++ b/lib/transport/webdriver.js @@ -53,6 +53,8 @@ class WebdriverProtocol extends Transport { errorMessage = result.value.message; } else if (result.value && result.value.error && this.Errors.Response[result.value.error]) { errorMessage = this.Errors.Response[result.value.error].message; + } else if (result.error) { + errorMessage = result.error; } return { diff --git a/test/lib/mockserver.js b/test/lib/mockserver.js index 7e1f1d2bf0..fa29ea64cc 100644 --- a/test/lib/mockserver.js +++ b/test/lib/mockserver.js @@ -11,7 +11,7 @@ class MockServer { postdata: '', weakURLVerification: false, responseHeaders: {}, - responseType: 'application/json', + contentType: 'application/json', mocks: [], finishedCallback() {} }; @@ -56,7 +56,7 @@ class MockServer { if (item) { headers = item.responseHeaders; responsedata = JSON.stringify(item.response); - headers['Content-Type'] = this.options.responseType; + headers['Content-Type'] = item.contentType || this.options.contentType; headers['Content-Length'] = responsedata.length; res.writeHead(Number(item.statusCode), headers); @@ -98,9 +98,22 @@ class MockServer { item.__once = true; } + // if (item.response && typeof item.response == 'string') { + // item.response = JSON.parse(item.response); + // } + if (item.response && typeof item.response == 'string') { - item.response = JSON.parse(item.response); + item.contentType = item.contentType || 'application/json'; + + if (item.contentType === 'application/json') { + try { + item.response = JSON.parse(item.response); + } catch (err) { + console.warn('Invalid json supplied as response:', item.response); + } + } } + this.mocks.push(item); } diff --git a/test/sampletests/withservererrors/sampleTestWithServerError.js b/test/sampletests/withservererrors/sampleTestWithServerError.js new file mode 100644 index 0000000000..a2ca7f8403 --- /dev/null +++ b/test/sampletests/withservererrors/sampleTestWithServerError.js @@ -0,0 +1,19 @@ +module.exports = { + demoTest(client) { + client + .url('http://localhost') + .waitForElementPresent('#weblogin') + .assert.not.elementPresent('#element-server-error') + }, + + demoTest2(client) { + client + .url('http://localhost') + .waitForElementPresent('#weblogin') + .waitForElementNotPresent('#element-server-error') + }, + + after(client) { + client.end(); + } +}; diff --git a/test/src/element/testElementCommands.js b/test/src/element/testElementCommands.js index dec1faafd7..7ce3e4d13f 100644 --- a/test/src/element/testElementCommands.js +++ b/test/src/element/testElementCommands.js @@ -126,6 +126,48 @@ describe('element base commands', function() { return Nightwatch.start(); }); + + it('client.element() with 502 gateway error', async function() { + Nightwatch.addMock({ + url : '/session/13521-10219-202/element', + postdata: { + using: 'css selector', + value: '#weblogin-error' + }, + contentType: 'text/plain', + statusCode: 502, + response: ` + +502 Bad Gateway + + +` + }, true); + + await Nightwatch.initAsync({ + output: false, + silent: false, + selenium : { + start_process: false, + }, + webdriver:{ + start_process: true + }, + }); + + Nightwatch.api().element('css selector', '#weblogin-error', function(result) { + assert.strictEqual(result.status, -1); + assert.strictEqual(result.httpStatusCode, 502); + assert.strictEqual(result.error, '\n\n502 Bad Gateway\n\n\n'); + assert.deepStrictEqual(result.value, { + error: 'internal server error', + message: '\n\n502 Bad Gateway\n\n\n' + }); + }); + + return Nightwatch.start(); + }); + ////////////////////////////////////////////////////////////////////////////////////// // .elements() ////////////////////////////////////////////////////////////////////////////////////// diff --git a/test/src/index/testNightwatchIndex.js b/test/src/index/testNightwatchIndex.js index 8685abd208..67f8ac0d74 100644 --- a/test/src/index/testNightwatchIndex.js +++ b/test/src/index/testNightwatchIndex.js @@ -202,7 +202,10 @@ describe('test NightwatchIndex', function () { client.startSession().catch(err => { assert.ok(err instanceof Error); assert.equal(typeof err.data, 'string'); - assert.deepEqual(JSON.parse(err.data), {message: 'Could not find device : iPhone 6', error: []}); + assert.deepEqual(JSON.parse(err.data), { + message: 'Could not find device : iPhone 6', + error: [] + }); assert.ok(err.message.indexOf('Could not find device : iPhone 6') > 0); done(); }).catch(err => done(err)); diff --git a/test/src/runner/cli/testParallelExecution.js b/test/src/runner/cli/testParallelExecution.js index 4c46f67984..add09da460 100644 --- a/test/src/runner/cli/testParallelExecution.js +++ b/test/src/runner/cli/testParallelExecution.js @@ -94,7 +94,7 @@ describe('test Parallel Execution', function() { assert.ok(runner.test_settings.test_workers); return runner.runTests().then(_ => { - assert.strictEqual(allArgs.length, 43); + assert.strictEqual(allArgs.length, 44); assert.strictEqual(runner.concurrency.globalExitCode, 0); }); }); @@ -136,7 +136,7 @@ describe('test Parallel Execution', function() { }); return runner.runTests().then(_ => { - assert.strictEqual(allArgs.length, 43); + assert.strictEqual(allArgs.length, 44); }); }); @@ -165,7 +165,7 @@ describe('test Parallel Execution', function() { }); return runner.runTests().then(_ => { - assert.strictEqual(allArgs.length, 43); + assert.strictEqual(allArgs.length, 44); }); }); diff --git a/test/src/runner/testRunWithServerErrors.js b/test/src/runner/testRunWithServerErrors.js new file mode 100644 index 0000000000..a99e40ed4b --- /dev/null +++ b/test/src/runner/testRunWithServerErrors.js @@ -0,0 +1,85 @@ +const path = require('path'); +const assert = require('assert'); +const common = require('../../common.js'); +const CommandGlobals = require('../../lib/globals/commands.js'); +const MockServer = require('../../lib/mockserver.js'); +const NightwatchClient = common.require('index.js'); + +describe('testRunWithServerErrors', function() { + + before(function(done) { + this.server = MockServer.init(); + + this.server.on('listening', () => { + done(); + }); + }); + + after(function(done) { + CommandGlobals.afterEach.call(this, done); + }); + + beforeEach(function() { + process.removeAllListeners('exit'); + process.removeAllListeners('uncaughtException'); + process.removeAllListeners('unhandledRejection'); + }); + + afterEach(function() { + Object.keys(require.cache).forEach(function(module) { + delete require.cache[module]; + }); + }); + + it('testRunner with 502 server errors', function() { + MockServer.addMock({ + url: '/wd/hub/session/1352110219202/elements', + postdata: { + using: 'css selector', + value: '#element-server-error' + }, + statusCode: 502, + contentType: 'text/html', + response: '\n\n502 Bad Gateway\n\n\n' + }, false, true); + + let testsPath = path.join(__dirname, '../../sampletests/withservererrors'); + let globals = { + calls: 0, + retryAssertionTimeout: 150, + waitForConditionTimeout: 150, + waitForConditionPollInterval: 50, + reporter(results, cb) { + assert.deepEqual(results.errmessages, []); + assert.strictEqual(results.passed, 2); + assert.strictEqual(results.failed, 2); + assert.strictEqual(results.assertions, 4); + assert.strictEqual(results.errors, 0); + assert.strictEqual(results.skipped, 0); + assert.ok(/Server Error: \s+502 Bad Gateway\s+/.test(results.modules.sampleTestWithServerError.completed.demoTest.lastError.message)); + assert.ok(/Server Error: \s+502 Bad Gateway\s+/.test(results.modules.sampleTestWithServerError.completed.demoTest2.lastError.message)); + cb(); + } + }; + + let settings = { + selenium: { + port: 10195, + version2: true, + start_process: true + }, + skip_testcases_on_fail: false, + output: false, + silent: false, + persist_globals: true, + disable_error_log: 0, + globals, + output_folder: false + }; + + return NightwatchClient.runTests({ + _source: [testsPath] + }, settings); + }); + +}); diff --git a/test/src/runner/testRunnerSessionCreate.js b/test/src/runner/testRunnerSessionCreate.js index e66d3eada8..a8a5c6d19a 100644 --- a/test/src/runner/testRunnerSessionCreate.js +++ b/test/src/runner/testRunnerSessionCreate.js @@ -96,7 +96,7 @@ describe('testRunnerSessionCreate', function() { webdriver: { start_process: true }, - output: process.env.VERBOSE === '1' || false, + output: false, silent: false, globals: globals, output_folder: false