Skip to content

Commit

Permalink
fix(update): Error handling esp. for chromedriver
Browse files Browse the repository at this point in the history 
Fail when requesting chromedriver version that isn't in the manifest.
Don't swallow downloader errors.
Fail loudly if trying to download a Binary with no URL.
Exit with nonzero status on update error.
Update to latest Jasmine for improved Promise handling in specs.
  • Loading branch information
@thw0rted
thw0rted committed Mar 17, 2021
1 parent 8415ad5 commit daffba4
Show file tree
Hide file tree
Showing 13 changed files with 322 additions and 327 deletions.
9 changes: 7 additions & 2 deletions lib/binaries/chrome_xml.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,17 @@ export class ChromeXml extends XmlConfigSource {

/**
* Gets a specific item from the XML.
*
* Resolves with URL, or rejects if there is an error retrieving the XML, or the requested
* was not found in the list.
*/
private getSpecificChromeDriverVersion(inputVersion: string): Promise<BinaryUrl> {
return this.getVersionList().then(list => {
const specificVersion = getValidSemver(inputVersion);
if (specificVersion === '') {
throw new Error(`version ${inputVersion} ChromeDriver does not exist`)
const msg = `Requested version ${inputVersion} does not look like a Chrome version.
Expected e.g. "2.34" or "89.0.4389.0".`;
throw new Error(msg);
}
let itemFound = '';
for (let item of list) {
Expand Down Expand Up @@ -117,7 +122,7 @@ export class ChromeXml extends XmlConfigSource {
}
}
if (itemFound == '') {
return {url: '', version: inputVersion};
throw new Error(`There is no chromedriver available for version ${specificVersion}.`);
} else {
return {url: Config.cdnUrls().chrome + itemFound, version: inputVersion};
}
Expand Down
20 changes: 16 additions & 4 deletions lib/cli/programs.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as minimist from 'minimist';
import {Logger} from './logger';
import {MinimistArgs, Option, Options} from './options';

import {Args, MinimistArgs, Option, Options} from './options';

const logger = new Logger('program');

/**
* Dictionary that maps the command and the program.
Expand Down Expand Up @@ -71,11 +72,22 @@ export class Program {
* method.
* @param args The arguments that will be parsed to run the method.
*/
run(json: JSON): Promise<void> {
run(json: JSON): void;
run(json: JSON, testing: true): Promise<void>;
run(json: JSON, testing?: true): void|Promise<void> {
for (let opt in this.options) {
this.options[opt].value = this.getValue_(opt, json);
}
return Promise.resolve(this.runMethod(this.options));
const promise = Promise.resolve(this.runMethod(this.options));
if (testing) {
return promise;
} else {
promise.catch(err => {
// Exit gracefully when the program promise rejects
logger.error(err);
process.exit(-1);
});
}
}

private getValue_(key: string, json: JSON): number|boolean|string {
Expand Down
70 changes: 25 additions & 45 deletions lib/cmds/update.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,51 +125,42 @@ function update(options: Options): Promise<void> {
if (standalone) {
let binary: Standalone = binaries[Standalone.id];
binary.versionCustom = options[Opt.VERSIONS_STANDALONE].getString();
promises.push(FileManager.downloadFile(binary, outputDir)
.then<void>((downloaded: boolean) => {
if (!downloaded) {
logger.info(
binary.name + ': file exists ' +
path.resolve(outputDir, binary.filename()));
logger.info(binary.name + ': ' + binary.filename() + ' up to date');
}
})
.then(() => {
updateBrowserFile(binary, outputDir);
}));
promises.push(FileManager.downloadFile(binary, outputDir).then(downloaded => {
if (!downloaded) {
logger.info(binary.name + ': file exists ' + path.resolve(outputDir, binary.filename()));
logger.info(binary.name + ': ' + binary.filename() + ' up to date');
}
updateBrowserFile(binary, outputDir);
}));
}
if (chrome) {
let binary: ChromeDriver = binaries[ChromeDriver.id];
binary.versionCustom = options[Opt.VERSIONS_CHROME].getString();
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL).then(() => {
return Promise.resolve(updateBrowserFile(binary, outputDir));
}));
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL)
.then(() => updateBrowserFile(binary, outputDir)));
}
if (gecko) {
let binary: GeckoDriver = binaries[GeckoDriver.id];
if (options[Opt.VERSIONS_GECKO]) {
binary.versionCustom = options[Opt.VERSIONS_GECKO].getString();
}
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL).then(() => {
return Promise.resolve(updateBrowserFile(binary, outputDir));
}));
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL)
.then(() => updateBrowserFile(binary, outputDir)));
}
if (ie64) {
let binary: IEDriver = binaries[IEDriver.id];
if (options[Opt.VERSIONS_IE]) {
binary.versionCustom = options[Opt.VERSIONS_IE].getString();
}
binary.osarch = Config.osArch(); // Win32 or x64
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL).then(() => {
return Promise.resolve(updateBrowserFile(binary, outputDir));
}));
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL)
.then(() => updateBrowserFile(binary, outputDir)));
}
if (ie32) {
let binary: IEDriver = binaries[IEDriver.id];
binary.osarch = 'Win32';
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL).then(() => {
return Promise.resolve(updateBrowserFile(binary, outputDir));
}));
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL)
.then(() => updateBrowserFile(binary, outputDir)));
}
if (android) {
let binary = binaries[AndroidSDK.id];
Expand All @@ -178,24 +169,15 @@ function update(options: Options): Promise<void> {
let oldAVDList: string;

updateBrowserFile(binary, outputDir);
promises.push(q.nfcall(fs.readFile, path.resolve(sdk_path, 'available_avds.json'))
.then(
(oldAVDs: string) => {
oldAVDList = oldAVDs;
},
() => {
oldAVDList = '[]';
})
.then(() => {
return updateBinary(binary, outputDir, proxy, ignoreSSL);
})
.then<void>(() => {
initializeAndroid(
path.resolve(outputDir, binary.executableFilename()),
android_api_levels, android_architectures, android_platforms,
android_accept_licenses, binary.versionCustom,
JSON.parse(oldAVDList), logger, verbose);
}));
promises.push(
q.nfcall(fs.readFile, path.resolve(sdk_path, 'available_avds.json'))
.then((oldAVDs: string) => oldAVDList = oldAVDs, () => oldAVDList = '[]')
.then(() => updateBinary(binary, outputDir, proxy, ignoreSSL))
.then(
() => initializeAndroid(
path.resolve(outputDir, binary.executableFilename()), android_api_levels,
android_architectures, android_platforms, android_accept_licenses,
binary.versionCustom, JSON.parse(oldAVDList), logger, verbose)));
}
if (ios) {
checkIOS(logger);
Expand All @@ -207,9 +189,7 @@ function update(options: Options): Promise<void> {
updateBrowserFile(binary, outputDir);
}

return Promise.all(promises).then(() => {
writeBrowserFile(outputDir);
});
return Promise.all(promises as Promise<void>[]).then(() => writeBrowserFile(outputDir));
}

function updateBinary<T extends Binary>(
Expand Down
121 changes: 58 additions & 63 deletions lib/files/downloader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,70 +40,65 @@ export class Downloader {
let resContentLength: number;

return new Promise<boolean>((resolve, reject) => {
req = request(options);
req.on('response', response => {
if (response.statusCode === 200) {
resContentLength = +response.headers['content-length'];
if (contentLength === resContentLength) {
// if the size is the same, do not download and stop here
response.destroy();
resolve(false);
} else {
let curl = outputDir + '/' + fileName + ' ' + options.url;
if (HttpUtils.requestOpts.proxy) {
let pathUrl = url.parse(options.url.toString()).path;
let host = url.parse(options.url.toString()).host;
let newFileUrl = url.resolve(HttpUtils.requestOpts.proxy, pathUrl);
curl = outputDir + '/' + fileName + ' \'' + newFileUrl +
'\' -H \'host:' + host + '\'';
}
if (HttpUtils.requestOpts.ignoreSSL) {
curl = 'k ' + curl;
}
logger.info('curl -o' + curl);
req = request(options);
req.on('response', response => {
if (response.statusCode === 200) {
resContentLength = +response.headers['content-length'];
if (contentLength === resContentLength) {
// if the size is the same, do not download and stop here
response.destroy();
resolve(false);
} else {
let curl = outputDir + '/' + fileName + ' ' + options.url;
if (HttpUtils.requestOpts.proxy) {
let pathUrl = url.parse(options.url.toString()).path;
let host = url.parse(options.url.toString()).host;
let newFileUrl = url.resolve(HttpUtils.requestOpts.proxy, pathUrl);
curl =
outputDir + '/' + fileName + ' \'' + newFileUrl + '\' -H \'host:' + host + '\'';
}
if (HttpUtils.requestOpts.ignoreSSL) {
curl = 'k ' + curl;
}
logger.info('curl -o' + curl);

// only pipe if the headers are different length
file = fs.createWriteStream(filePath);
req.pipe(file);
file.on('close', () => {
fs.stat(filePath, (error, stats) => {
if (error) {
(error as any).msg = 'Error: Got error ' + error + ' from ' + fileUrl;
return reject(error);
}
if (stats.size != resContentLength) {
(error as any).msg = 'Error: corrupt download for ' + fileName +
'. Please re-run webdriver-manager update';
fs.unlinkSync(filePath);
reject(error);
}
if (callback) {
callback(binary, outputDir, fileName);
}
resolve(true);
});
});
}
// only pipe if the headers are different length
file = fs.createWriteStream(filePath);
req.pipe(file);
file.on('close', () => {
fs.stat(filePath, (error, stats) => {
if (error) {
(error as any).msg = 'Error: Got error ' + error + ' from ' + fileUrl;
return reject(error);
}
if (stats.size != resContentLength) {
(error as any).msg = 'Error: corrupt download for ' + fileName +
'. Please re-run webdriver-manager update';
fs.unlinkSync(filePath);
reject(error);
}
if (callback) {
callback(binary, outputDir, fileName);
}
resolve(true);
});
});
}

} else {
let error = new Error();
(error as any).msg =
'Expected response code 200, received: ' + response.statusCode;
reject(error);
}
});
req.on('error', error => {
if ((error as any).code === 'ETIMEDOUT') {
(error as any).msg = 'Connection timeout downloading: ' + fileUrl +
'. Default timeout is 4 minutes.';
} else if ((error as any).connect) {
(error as any).msg = 'Could not connect to the server to download: ' + fileUrl;
}
reject(error);
});
})
.catch(error => {
logger.error((error as any).msg || (error as any).message);
});
} else {
let error = new Error('Expected response code 200, received: ' + response.statusCode);
reject(error);
}
});
req.on('error', error => {
if ((error as any).code === 'ETIMEDOUT') {
(error as any).msg =
'Connection timeout downloading: ' + fileUrl + '. Default timeout is 4 minutes.';
} else if ((error as any).connect) {
(error as any).msg = 'Could not connect to the server to download: ' + fileUrl;
}
reject(error);
});
});
}
}
62 changes: 28 additions & 34 deletions lib/files/file_manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,42 +171,36 @@ export class FileManager {
*/
static downloadFile<T extends Binary>(binary: T, outputDir: string, callback?: Function):
Promise<boolean> {
return new Promise<boolean>((resolve, reject) => {
let outDir = Config.getSeleniumDir();
let downloaded: BinaryMap<DownloadedBinary> = FileManager.downloadedBinaries(outputDir);
let contentLength = 0;

// Pass options down to binary to make request to get the latest version to download.
binary.getUrl(binary.version()).then(fileUrl => {
binary.versionCustom = fileUrl.version;
let filePath = path.resolve(outputDir, binary.filename());
let fileName = binary.filename();

// If we have downloaded the file before, check the content length
if (downloaded[binary.id()]) {
let downloadedBinary = downloaded[binary.id()];
let versions = downloadedBinary.versions;
let version = binary.versionCustom;

for (let index in versions) {
let v = versions[index];
if (v === version) {
contentLength = fs.statSync(filePath).size;

Downloader.getFile(binary, fileUrl.url, fileName, outputDir, contentLength, callback)
.then(downloaded => {
resolve(downloaded);
});
}
const downloaded: BinaryMap<DownloadedBinary> = FileManager.downloadedBinaries(outputDir);

// Pass options down to binary to make request to get the latest version to download.
return binary.getUrl(binary.version()).then(fileUrl => {
const url = fileUrl.url;
if (!url) {
logger.debug('Bad binary entry from FileManager', binary);
throw new Error('Developer error: tried to download a binary with no URL.');
}
const version = binary.versionCustom = fileUrl.version;
const filePath = path.resolve(outputDir, binary.filename());
const fileName = binary.filename();

// If we have downloaded the file before, check the content length
if (downloaded[binary.id()]) {
const downloadedBinary = downloaded[binary.id()];
const versions = downloadedBinary.versions;

for (let index in versions) {
let v = versions[index];
if (v === version) {
const contentLength = fs.statSync(filePath).size;
return Downloader.getFile(binary, url, fileName, outputDir, contentLength, callback);
}
}
// We have not downloaded it before, or the version does not exist. Use the default content
// length of zero and download the file.
Downloader.getFile(binary, fileUrl.url, fileName, outputDir, contentLength, callback)
.then(downloaded => {
resolve(downloaded);
});
});
}

// We have not downloaded it before, or the version does not exist. Use the default content
// length of zero and download the file.
return Downloader.getFile(binary, url, fileName, outputDir, 0, callback)
});
}

Expand Down
7 changes: 3 additions & 4 deletions lib/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ function spawnFactory(sync: true):
(cmd: string, args: string[], stdio?: any, opts?: child_process.SpawnSyncOptions) =>
child_process.SpawnSyncReturns<any>;
function spawnFactory(sync: boolean):
(cmd: string, args: string[], stdio?: string,
(cmd: string, args: string[], stdio?: child_process.StdioOptions,
opts?: child_process.SpawnOptions|child_process.SpawnSyncOptions) =>
child_process.ChildProcess | child_process.SpawnSyncReturns<any> {
return (cmd: string, args: string[], stdio?: string,
opts?: child_process.SpawnOptions|child_process.SpawnSyncOptions) => {
return (cmd: string, args: string[], stdio?: child_process.StdioOptions,
opts: child_process.SpawnOptions|child_process.SpawnSyncOptions = {}) => {
if ((Config.osType() === 'Windows_NT') && (cmd.slice(-4) !== '.exe')) {
if (fs.existsSync(cmd + '.exe')) {
cmd += '.exe';
Expand All @@ -26,7 +26,6 @@ function spawnFactory(sync: boolean):
}
}
if (stdio) {
opts = opts || {};
opts.stdio = stdio;
}
if (sync) {
Expand Down
Loading

0 comments on commit daffba4

Please sign in to comment.