Skip to content

Commit 1e636a6

Browse files
committed
fix(puppet): no chrome launch errors to client
1 parent 19351b5 commit 1e636a6

File tree

10 files changed

+88
-44
lines changed

10 files changed

+88
-44
lines changed

core/server/ConnectionToClient.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import IWaitForOptions from '@secret-agent/core-interfaces/IWaitForOptions';
1111
import IAgentMeta from '@secret-agent/core-interfaces/IAgentMeta';
1212
import Log from '@secret-agent/commons/Logger';
1313
import { CanceledPromiseError } from '@secret-agent/commons/interfaces/IPendingWaitEvent';
14+
import PuppetLaunchError from '@secret-agent/puppet/lib/PuppetLaunchError';
15+
import { DependenciesMissingError } from '@secret-agent/puppet/lib/DependenciesMissingError';
1416
import Session from '../lib/Session';
1517
import Tab from '../lib/Tab';
1618
import GlobalPool from '../lib/GlobalPool';
@@ -65,15 +67,28 @@ export default class ConnectionToClient extends TypedEventEmitter<{
6567
if ((this.isClosing || session?.isClosing) && error instanceof CanceledPromiseError) {
6668
return;
6769
}
70+
const isChildProcess = !!process.send;
71+
if (isChildProcess === false) {
72+
log.error('ConnectionToClient.HandleRequestError', {
73+
error,
74+
sessionId: session?.id,
75+
});
76+
}
6877
isError = true;
69-
data =
70-
error instanceof Error
71-
? {
72-
message: error.message,
73-
stack: error.stack,
74-
...error,
75-
}
76-
: new Error(`Unknown error occurred ${error}`);
78+
if (error instanceof PuppetLaunchError || error instanceof DependenciesMissingError) {
79+
data = {
80+
message: 'CoreServer needs further setup to launch the browserEmulator. See server logs.',
81+
};
82+
} else if (error instanceof Error) {
83+
data = {
84+
message: error.message,
85+
stack: error.stack,
86+
...error,
87+
};
88+
} else {
89+
const tempError = new Error(`Unknown error occurred ${error}`);
90+
data = { message: tempError.message, stack: tempError.stack };
91+
}
7792
}
7893

7994
const commandId = session?.sessionState?.lastCommand?.id;

puppet-chrome/index.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import IPuppetLauncher from '@secret-agent/puppet-interfaces/IPuppetLauncher';
44
import * as os from 'os';
55
import * as Path from 'path';
66
import IBrowserEngine from '@secret-agent/core-interfaces/IBrowserEngine';
7+
import { IPuppetLaunchError } from '@secret-agent/puppet-interfaces/IPuppetLaunchError';
78
import { Browser } from './lib/Browser';
89
import { Connection } from './lib/Connection';
910

@@ -51,29 +52,35 @@ const PuppetLauncher: IPuppetLauncher = {
5152
const connection = new Connection(transport);
5253
return await Browser.create(connection, engine, close);
5354
} catch (error) {
54-
close();
55+
await close();
5556
throw error;
5657
}
5758
},
58-
translateLaunchError(error: Error): Error {
59+
translateLaunchError(rawError: Error): IPuppetLaunchError {
5960
// These error messages are taken from Chromium source code as of July, 2020:
6061
// https://github.com/chromium/chromium/blob/70565f67e79f79e17663ad1337dc6e63ee207ce9/content/browser/zygote_host/zygote_host_impl_linux.cc
62+
const error = {
63+
message: rawError.message,
64+
stack: rawError.stack,
65+
name: 'PuppetLaunchError',
66+
isSandboxError: false,
67+
};
6168
if (
62-
!error.message.includes('crbug.com/357670') &&
63-
!error.message.includes('No usable sandbox!') &&
64-
!error.message.includes('crbug.com/638180')
69+
error.message.includes('crbug.com/357670') ||
70+
error.message.includes('No usable sandbox!') ||
71+
error.message.includes('crbug.com/638180')
6572
) {
66-
return error;
73+
error.stack += [
74+
`\nChromium sandboxing failed!`,
75+
`================================`,
76+
`To workaround sandboxing issues, do either of the following:`,
77+
` - (preferred): Configure environment to support sandboxing (as here: https://github.com/ulixee/secret-agent/tree/master/tools/docker)`,
78+
` - (alternative): Launch Chromium without sandbox using 'NO_CHROME_SANDBOX=false' environmental variable`,
79+
`================================`,
80+
``,
81+
].join('\n');
82+
error.isSandboxError = true;
6783
}
68-
error.stack += [
69-
`\nChromium sandboxing failed!`,
70-
`================================`,
71-
`To workaround sandboxing issues, do either of the following:`,
72-
` - (preferred): Configure environment to support sandboxing (eg: in Docker, use custom seccomp profile + non-root user + --ipc=host)`,
73-
` - (alternative): Launch Chromium without sandbox using 'NO_CHROME_SANDBOX=false' environmental variable`,
74-
`================================`,
75-
``,
76-
].join('\n');
7784
return error;
7885
},
7986
};

puppet-chrome/lib/CDPSession.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ export class CDPSession extends EventEmitter {
115115
error.stack += `\n${'------DEVTOOLS'.padEnd(
116116
50,
117117
'-',
118-
)}\n${`------DEVTOOLS-SESSION-ID =${this.sessionId}`.padEnd(50, '-')}\n${resolvable.stack}`;
118+
)}\n${`------CDP_SESSION_ID=${this.sessionId}`.padEnd(50, '-')}\n${resolvable.stack}`;
119119
resolvable.reject(error);
120120
}
121121
this.pendingMessages.clear();
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export interface IPuppetLaunchError extends Error {
2+
isSandboxError: boolean;
3+
}
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import IBrowserEngine from '@secret-agent/core-interfaces/IBrowserEngine';
22
import ILaunchedProcess from './ILaunchedProcess';
33
import IPuppetBrowser from './IPuppetBrowser';
4+
import { IPuppetLaunchError } from './IPuppetLaunchError';
45

56
export default interface IPuppetLauncher {
67
getLaunchArgs(options: { proxyPort?: number; showBrowser?: boolean }): string[];
78
createPuppet(process: ILaunchedProcess, engine: IBrowserEngine): Promise<IPuppetBrowser>;
8-
translateLaunchError(error: Error): Error;
9+
translateLaunchError(error: Error): IPuppetLaunchError;
910
}

puppet/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { existsSync } from 'fs';
99
import launchProcess from './lib/launchProcess';
1010
import { validateHostRequirements } from './lib/validateHostDependencies';
1111
import { EngineFetcher } from './lib/EngineFetcher';
12+
import PuppetLaunchError from './lib/PuppetLaunchError';
1213

1314
const { log } = Log(module);
1415

@@ -100,7 +101,12 @@ export default class Puppet {
100101
const launchedProcess = await launchProcess(executablePath, launchArgs, {}, pipeBrowserIo);
101102
return launcher.createPuppet(launchedProcess, this.engine);
102103
} catch (err) {
103-
throw launcher.translateLaunchError(err);
104+
const launchError = launcher.translateLaunchError(err);
105+
throw new PuppetLaunchError(
106+
launchError.message,
107+
launchError.stack,
108+
launchError.isSandboxError,
109+
);
104110
}
105111
}
106112

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export class DependenciesMissingError extends Error {
2+
constructor(
3+
resolutionMessage: string,
4+
readonly engineName: string,
5+
readonly missingDependencies: string[],
6+
) {
7+
super(
8+
`Some of the dependencies needed to run ${engineName} are not on your system!\n\n${resolutionMessage}`,
9+
);
10+
}
11+
}

puppet/lib/PuppetLaunchError.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { IPuppetLaunchError } from '@secret-agent/puppet-interfaces/IPuppetLaunchError';
2+
3+
export default class PuppetLaunchError extends Error implements IPuppetLaunchError {
4+
constructor(message: string, stack: string, readonly isSandboxError: boolean) {
5+
super(message);
6+
this.stack = stack;
7+
}
8+
}

puppet/lib/installEngineWithProgress.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ export default async function installEngineWithProgress(engine: IBrowserEngineCo
4040
progressBar.tick(delta);
4141
});
4242

43-
await validateHostRequirements(engineFetcher.toJSON());
43+
// don't blow up during install process if host requirements can't be met
44+
await validateHostRequirements(engineFetcher.toJSON()).catch(err => npmlog(err.toString()));
4445

4546
npmlog(`${browserTitle} (${version}) downloaded to ${engineFetcher.browsersDir}`);
4647
} catch (error) {

puppet/lib/validateHostDependencies.ts

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,9 @@ import * as path from 'path';
2020
import * as os from 'os';
2121
import { spawn } from 'child_process';
2222
import IBrowserEngine from '@secret-agent/core-interfaces/IBrowserEngine';
23-
import Log from '@secret-agent/commons/Logger';
2423
import { isDebianFlavor } from './LinuxUtils';
2524
import DependencyInstaller from './DependencyInstaller';
26-
27-
const { log } = Log(module);
25+
import { DependenciesMissingError } from './DependenciesMissingError';
2826

2927
export async function validateHostRequirements(engine: IBrowserEngine): Promise<void> {
3028
const isWindows64 = os.platform() === 'win32' && os.arch() === 'x64';
@@ -48,7 +46,7 @@ export async function validateHostRequirements(engine: IBrowserEngine): Promise<
4846
resolutionMessage = getWindowsResolutionMessage(missingDeps);
4947
} else if (isDebian) {
5048
await DependencyInstaller.appendAptInstallNeeded(engine);
51-
resolutionMessage = `You can resolve this by running the apt installer at:
49+
resolutionMessage = `You can resolve this by running the apt dependency installer at:
5250
-------------------------------------------------
5351
5452
${DependencyInstaller.aptScriptPath}
@@ -62,9 +60,7 @@ missing: ${[...missingDeps].join(', ')}
6260
'\n ',
6361
)}`;
6462
}
65-
log.warn(
66-
`Some of the dependencies needed to run ${engineName} are not on your system!\n\n${resolutionMessage}`,
67-
);
63+
throw new DependenciesMissingError(resolutionMessage, engineName, [...missingDeps]);
6864
}
6965

7066
function getWindowsResolutionMessage(missingDeps: Set<string>): string {
@@ -122,21 +118,17 @@ async function findAllMissingDependencies(directoryPath: string): Promise<Set<st
122118
return await spawnMissingDepsCheck(filePath);
123119
}
124120

125-
const canAccess = await fileExists(filePath, FsConstants.X_OK);
126-
if (canAccess) {
121+
try {
122+
await Fs.access(filePath, FsConstants.X_OK);
127123
return await spawnMissingDepsCheck(filePath);
124+
} catch (error) {
125+
// just break through and return if we can't access
128126
}
129127
return [];
130128
}),
131129
);
132130

133-
return new Set<string>(...[].concat(...missingDeps));
134-
}
135-
136-
function fileExists(filePath: string, mode?: number): Promise<boolean> {
137-
return Fs.access(filePath, mode)
138-
.then(() => true)
139-
.catch(() => false);
131+
return new Set<string>([].concat(...missingDeps));
140132
}
141133

142134
async function spawnMissingDepsCheck(filePath: string): Promise<string[]> {
@@ -168,7 +160,7 @@ async function spawnMissingDepsCheck(filePath: string): Promise<string[]> {
168160
}
169161

170162
return stdout
171-
.split(/\r?\n/)
163+
.split(/\r?\n/g)
172164
.filter(line => line.trim().endsWith('not found') && line.includes('=>'))
173165
.map(line => line.split('=>')[0].trim());
174166
}

0 commit comments

Comments
 (0)