Skip to content

Commit 57bd209

Browse files
thw0rtedthw0rted
committed
fix(update): Error handling esp. for chromedriver
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.
1 parent 8415ad5 commit 57bd209

13 files changed

+321
-312
lines changed

‎lib/binaries/chrome_xml.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,15 @@ export class ChromeXml extends XmlConfigSource {
7373

7474
/**
7575
* Gets a specific item from the XML.
76+
*
77+
* Resolves with URL, or rejects if there is an error retrieving the XML, or the requested
78+
* was not found in the list.
7679
*/
7780
private getSpecificChromeDriverVersion(inputVersion: string): Promise<BinaryUrl> {
7881
return this.getVersionList().then(list => {
7982
const specificVersion = getValidSemver(inputVersion);
8083
if (specificVersion === '') {
81-
throw new Error(`version ${inputVersion} ChromeDriver does not exist`)
84+
throw new Error(`Requested version ${inputVersion} does not look like a Chrome version. Expected e.g. "2.34" or "89.0.4389.0".`)
8285
}
8386
let itemFound = '';
8487
for (let item of list) {
@@ -117,7 +120,7 @@ export class ChromeXml extends XmlConfigSource {
117120
}
118121
}
119122
if (itemFound == '') {
120-
return {url: '', version: inputVersion};
123+
throw new Error(`There is no chromedriver available for version ${specificVersion}.`);
121124
} else {
122125
return {url: Config.cdnUrls().chrome + itemFound, version: inputVersion};
123126
}

‎lib/cli/programs.ts

+16-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import * as minimist from 'minimist';
1+
import {Logger} from './logger';
2+
import {MinimistArgs, Option, Options} from './options';
23

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

5+
const logger = new Logger('program');
56

67
/**
78
* Dictionary that maps the command and the program.
@@ -71,11 +72,22 @@ export class Program {
7172
* method.
7273
* @param args The arguments that will be parsed to run the method.
7374
*/
74-
run(json: JSON): Promise<void> {
75+
run(json: JSON): void;
76+
run(json: JSON, testing: true): Promise<void>;
77+
run(json: JSON, testing?: true): void|Promise<void> {
7578
for (let opt in this.options) {
7679
this.options[opt].value = this.getValue_(opt, json);
7780
}
78-
return Promise.resolve(this.runMethod(this.options));
81+
const promise = Promise.resolve(this.runMethod(this.options));
82+
if (testing) {
83+
return promise;
84+
} else {
85+
promise.catch(err => {
86+
// Exit gracefully when the program promise rejects
87+
logger.error(err);
88+
process.exit(-1);
89+
});
90+
}
7991
}
8092

8193
private getValue_(key: string, json: JSON): number|boolean|string {

‎lib/cmds/update.ts

+29-45
Original file line numberDiff line numberDiff line change
@@ -125,51 +125,46 @@ function update(options: Options): Promise<void> {
125125
if (standalone) {
126126
let binary: Standalone = binaries[Standalone.id];
127127
binary.versionCustom = options[Opt.VERSIONS_STANDALONE].getString();
128-
promises.push(FileManager.downloadFile(binary, outputDir)
129-
.then<void>((downloaded: boolean) => {
130-
if (!downloaded) {
131-
logger.info(
132-
binary.name + ': file exists ' +
133-
path.resolve(outputDir, binary.filename()));
134-
logger.info(binary.name + ': ' + binary.filename() + ' up to date');
135-
}
136-
})
137-
.then(() => {
138-
updateBrowserFile(binary, outputDir);
139-
}));
128+
promises.push(FileManager.downloadFile(binary, outputDir).then(downloaded => {
129+
if (!downloaded) {
130+
logger.info(binary.name + ': file exists ' + path.resolve(outputDir, binary.filename()));
131+
logger.info(binary.name + ': ' + binary.filename() + ' up to date');
132+
}
133+
updateBrowserFile(binary, outputDir);
134+
}));
140135
}
141136
if (chrome) {
142137
let binary: ChromeDriver = binaries[ChromeDriver.id];
143138
binary.versionCustom = options[Opt.VERSIONS_CHROME].getString();
144-
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL).then(() => {
145-
return Promise.resolve(updateBrowserFile(binary, outputDir));
146-
}));
139+
promises.push(
140+
updateBinary(binary, outputDir, proxy, ignoreSSL)
141+
.then(() => updateBrowserFile(binary, outputDir)));
147142
}
148143
if (gecko) {
149144
let binary: GeckoDriver = binaries[GeckoDriver.id];
150145
if (options[Opt.VERSIONS_GECKO]) {
151146
binary.versionCustom = options[Opt.VERSIONS_GECKO].getString();
152147
}
153-
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL).then(() => {
154-
return Promise.resolve(updateBrowserFile(binary, outputDir));
155-
}));
148+
promises.push(
149+
updateBinary(binary, outputDir, proxy, ignoreSSL)
150+
.then(() => updateBrowserFile(binary, outputDir)));
156151
}
157152
if (ie64) {
158153
let binary: IEDriver = binaries[IEDriver.id];
159154
if (options[Opt.VERSIONS_IE]) {
160155
binary.versionCustom = options[Opt.VERSIONS_IE].getString();
161156
}
162157
binary.osarch = Config.osArch(); // Win32 or x64
163-
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL).then(() => {
164-
return Promise.resolve(updateBrowserFile(binary, outputDir));
165-
}));
158+
promises.push(
159+
updateBinary(binary, outputDir, proxy, ignoreSSL)
160+
.then(() => updateBrowserFile(binary, outputDir)));
166161
}
167162
if (ie32) {
168163
let binary: IEDriver = binaries[IEDriver.id];
169164
binary.osarch = 'Win32';
170-
promises.push(updateBinary(binary, outputDir, proxy, ignoreSSL).then(() => {
171-
return Promise.resolve(updateBrowserFile(binary, outputDir));
172-
}));
165+
promises.push(
166+
updateBinary(binary, outputDir, proxy, ignoreSSL)
167+
.then(() => updateBrowserFile(binary, outputDir)));
173168
}
174169
if (android) {
175170
let binary = binaries[AndroidSDK.id];
@@ -178,24 +173,15 @@ function update(options: Options): Promise<void> {
178173
let oldAVDList: string;
179174

180175
updateBrowserFile(binary, outputDir);
181-
promises.push(q.nfcall(fs.readFile, path.resolve(sdk_path, 'available_avds.json'))
182-
.then(
183-
(oldAVDs: string) => {
184-
oldAVDList = oldAVDs;
185-
},
186-
() => {
187-
oldAVDList = '[]';
188-
})
189-
.then(() => {
190-
return updateBinary(binary, outputDir, proxy, ignoreSSL);
191-
})
192-
.then<void>(() => {
193-
initializeAndroid(
194-
path.resolve(outputDir, binary.executableFilename()),
195-
android_api_levels, android_architectures, android_platforms,
196-
android_accept_licenses, binary.versionCustom,
197-
JSON.parse(oldAVDList), logger, verbose);
198-
}));
176+
promises.push(
177+
q.nfcall(fs.readFile, path.resolve(sdk_path, 'available_avds.json'))
178+
.then((oldAVDs: string) => oldAVDList = oldAVDs, () => oldAVDList = '[]')
179+
.then(() => updateBinary(binary, outputDir, proxy, ignoreSSL))
180+
.then(
181+
() => initializeAndroid(
182+
path.resolve(outputDir, binary.executableFilename()), android_api_levels,
183+
android_architectures, android_platforms, android_accept_licenses,
184+
binary.versionCustom, JSON.parse(oldAVDList), logger, verbose)));
199185
}
200186
if (ios) {
201187
checkIOS(logger);
@@ -207,9 +193,7 @@ function update(options: Options): Promise<void> {
207193
updateBrowserFile(binary, outputDir);
208194
}
209195

210-
return Promise.all(promises).then(() => {
211-
writeBrowserFile(outputDir);
212-
});
196+
return Promise.all(promises as Promise<void>[]).then(() => writeBrowserFile(outputDir));
213197
}
214198

215199
function updateBinary<T extends Binary>(

‎lib/files/downloader.ts

+58-63
Original file line numberDiff line numberDiff line change
@@ -40,70 +40,65 @@ export class Downloader {
4040
let resContentLength: number;
4141

4242
return new Promise<boolean>((resolve, reject) => {
43-
req = request(options);
44-
req.on('response', response => {
45-
if (response.statusCode === 200) {
46-
resContentLength = +response.headers['content-length'];
47-
if (contentLength === resContentLength) {
48-
// if the size is the same, do not download and stop here
49-
response.destroy();
50-
resolve(false);
51-
} else {
52-
let curl = outputDir + '/' + fileName + ' ' + options.url;
53-
if (HttpUtils.requestOpts.proxy) {
54-
let pathUrl = url.parse(options.url.toString()).path;
55-
let host = url.parse(options.url.toString()).host;
56-
let newFileUrl = url.resolve(HttpUtils.requestOpts.proxy, pathUrl);
57-
curl = outputDir + '/' + fileName + ' \'' + newFileUrl +
58-
'\' -H \'host:' + host + '\'';
59-
}
60-
if (HttpUtils.requestOpts.ignoreSSL) {
61-
curl = 'k ' + curl;
62-
}
63-
logger.info('curl -o' + curl);
43+
req = request(options);
44+
req.on('response', response => {
45+
if (response.statusCode === 200) {
46+
resContentLength = +response.headers['content-length'];
47+
if (contentLength === resContentLength) {
48+
// if the size is the same, do not download and stop here
49+
response.destroy();
50+
resolve(false);
51+
} else {
52+
let curl = outputDir + '/' + fileName + ' ' + options.url;
53+
if (HttpUtils.requestOpts.proxy) {
54+
let pathUrl = url.parse(options.url.toString()).path;
55+
let host = url.parse(options.url.toString()).host;
56+
let newFileUrl = url.resolve(HttpUtils.requestOpts.proxy, pathUrl);
57+
curl =
58+
outputDir + '/' + fileName + ' \'' + newFileUrl + '\' -H \'host:' + host + '\'';
59+
}
60+
if (HttpUtils.requestOpts.ignoreSSL) {
61+
curl = 'k ' + curl;
62+
}
63+
logger.info('curl -o' + curl);
6464

65-
// only pipe if the headers are different length
66-
file = fs.createWriteStream(filePath);
67-
req.pipe(file);
68-
file.on('close', () => {
69-
fs.stat(filePath, (error, stats) => {
70-
if (error) {
71-
(error as any).msg = 'Error: Got error ' + error + ' from ' + fileUrl;
72-
return reject(error);
73-
}
74-
if (stats.size != resContentLength) {
75-
(error as any).msg = 'Error: corrupt download for ' + fileName +
76-
'. Please re-run webdriver-manager update';
77-
fs.unlinkSync(filePath);
78-
reject(error);
79-
}
80-
if (callback) {
81-
callback(binary, outputDir, fileName);
82-
}
83-
resolve(true);
84-
});
85-
});
86-
}
65+
// only pipe if the headers are different length
66+
file = fs.createWriteStream(filePath);
67+
req.pipe(file);
68+
file.on('close', () => {
69+
fs.stat(filePath, (error, stats) => {
70+
if (error) {
71+
(error as any).msg = 'Error: Got error ' + error + ' from ' + fileUrl;
72+
return reject(error);
73+
}
74+
if (stats.size != resContentLength) {
75+
(error as any).msg = 'Error: corrupt download for ' + fileName +
76+
'. Please re-run webdriver-manager update';
77+
fs.unlinkSync(filePath);
78+
reject(error);
79+
}
80+
if (callback) {
81+
callback(binary, outputDir, fileName);
82+
}
83+
resolve(true);
84+
});
85+
});
86+
}
8787

88-
} else {
89-
let error = new Error();
90-
(error as any).msg =
91-
'Expected response code 200, received: ' + response.statusCode;
92-
reject(error);
93-
}
94-
});
95-
req.on('error', error => {
96-
if ((error as any).code === 'ETIMEDOUT') {
97-
(error as any).msg = 'Connection timeout downloading: ' + fileUrl +
98-
'. Default timeout is 4 minutes.';
99-
} else if ((error as any).connect) {
100-
(error as any).msg = 'Could not connect to the server to download: ' + fileUrl;
101-
}
102-
reject(error);
103-
});
104-
})
105-
.catch(error => {
106-
logger.error((error as any).msg || (error as any).message);
107-
});
88+
} else {
89+
let error = new Error('Expected response code 200, received: ' + response.statusCode);
90+
reject(error);
91+
}
92+
});
93+
req.on('error', error => {
94+
if ((error as any).code === 'ETIMEDOUT') {
95+
(error as any).msg =
96+
'Connection timeout downloading: ' + fileUrl + '. Default timeout is 4 minutes.';
97+
} else if ((error as any).connect) {
98+
(error as any).msg = 'Could not connect to the server to download: ' + fileUrl;
99+
}
100+
reject(error);
101+
});
102+
});
108103
}
109104
}

‎lib/files/file_manager.ts

+28-34
Original file line numberDiff line numberDiff line change
@@ -171,42 +171,36 @@ export class FileManager {
171171
*/
172172
static downloadFile<T extends Binary>(binary: T, outputDir: string, callback?: Function):
173173
Promise<boolean> {
174-
return new Promise<boolean>((resolve, reject) => {
175-
let outDir = Config.getSeleniumDir();
176-
let downloaded: BinaryMap<DownloadedBinary> = FileManager.downloadedBinaries(outputDir);
177-
let contentLength = 0;
178-
179-
// Pass options down to binary to make request to get the latest version to download.
180-
binary.getUrl(binary.version()).then(fileUrl => {
181-
binary.versionCustom = fileUrl.version;
182-
let filePath = path.resolve(outputDir, binary.filename());
183-
let fileName = binary.filename();
184-
185-
// If we have downloaded the file before, check the content length
186-
if (downloaded[binary.id()]) {
187-
let downloadedBinary = downloaded[binary.id()];
188-
let versions = downloadedBinary.versions;
189-
let version = binary.versionCustom;
190-
191-
for (let index in versions) {
192-
let v = versions[index];
193-
if (v === version) {
194-
contentLength = fs.statSync(filePath).size;
195-
196-
Downloader.getFile(binary, fileUrl.url, fileName, outputDir, contentLength, callback)
197-
.then(downloaded => {
198-
resolve(downloaded);
199-
});
200-
}
174+
const downloaded: BinaryMap<DownloadedBinary> = FileManager.downloadedBinaries(outputDir);
175+
176+
// Pass options down to binary to make request to get the latest version to download.
177+
return binary.getUrl(binary.version()).then(fileUrl => {
178+
const url = fileUrl.url;
179+
if (!url) {
180+
logger.debug('Bad binary entry from FileManager', binary);
181+
throw new Error('Developer error: tried to download a binary with no URL.');
182+
}
183+
const version = binary.versionCustom = fileUrl.version;
184+
const filePath = path.resolve(outputDir, binary.filename());
185+
const fileName = binary.filename();
186+
187+
// If we have downloaded the file before, check the content length
188+
if (downloaded[binary.id()]) {
189+
const downloadedBinary = downloaded[binary.id()];
190+
const versions = downloadedBinary.versions;
191+
192+
for (let index in versions) {
193+
let v = versions[index];
194+
if (v === version) {
195+
const contentLength = fs.statSync(filePath).size;
196+
return Downloader.getFile(binary, url, fileName, outputDir, contentLength, callback);
201197
}
202198
}
203-
// We have not downloaded it before, or the version does not exist. Use the default content
204-
// length of zero and download the file.
205-
Downloader.getFile(binary, fileUrl.url, fileName, outputDir, contentLength, callback)
206-
.then(downloaded => {
207-
resolve(downloaded);
208-
});
209-
});
199+
}
200+
201+
// We have not downloaded it before, or the version does not exist. Use the default content
202+
// length of zero and download the file.
203+
return Downloader.getFile(binary, url, fileName, outputDir, 0, callback)
210204
});
211205
}
212206

0 commit comments

Comments
 (0)