Skip to content

Commit

Permalink
fix code according to review feedbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
@jyyi1
jyyi1 committed Oct 27, 2022
1 parent cdc6f05 commit 4dc8ff3
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 85 deletions.
80 changes: 44 additions & 36 deletions src/electron/go_vpn_tunnel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import {platform} from 'os';
import {pathToEmbeddedBinary} from '../infrastructure/electron/app_paths';
import {ShadowsocksSessionConfig} from '../www/app/tunnel';
import {TunnelStatus} from '../www/app/tunnel';
import * as errors from '../www/model/errors';
import {ErrorCode, fromErrorCode, UnexpectedPluginError} from '../www/model/errors';

import {ChildProcessHelper} from './process';
import {ChildProcessHelper, ProcessTerminatedExitCodeError, ProcessTerminatedSignalError} from './process';
import {RoutingDaemon} from './routing_service';
import {VpnTunnel} from './vpn_tunnel';

Expand Down Expand Up @@ -100,7 +100,10 @@ export class GoVpnTunnel implements VpnTunnel {
this.isUdpEnabled = await checkConnectivity(this.config);
}
console.log(`UDP support: ${this.isUdpEnabled}`);
this.tun2socks.start(this.isUdpEnabled);

// Don't await here because we want to launch both binaries
this.tun2socks.start(this.isUdpEnabled).finally(() => this.disconnect());

await this.routing.start();
}

Expand Down Expand Up @@ -175,7 +178,9 @@ export class GoVpnTunnel implements VpnTunnel {
try {
await this.tun2socks.stop();
} catch (e) {
console.error(`could not stop tun2socks: ${e.message}`);
if (!(e instanceof ProcessTerminatedSignalError)) {
console.error(`could not stop tun2socks: ${e.message}`);
}
}

try {
Expand Down Expand Up @@ -211,14 +216,14 @@ export class GoVpnTunnel implements VpnTunnel {
// outline-go-tun2socks is a Go program that processes IP traffic from a TUN/TAP device
// and relays it to a Shadowsocks proxy server.
class GoTun2socks {
private autoRestart = false;
private stopRequested = false;
private readonly process: ChildProcessHelper;

constructor(private readonly config: ShadowsocksSessionConfig) {
this.process = new ChildProcessHelper(pathToEmbeddedBinary('outline-go-tun2socks', 'tun2socks'));
}

start(isUdpEnabled: boolean): void {
async start(isUdpEnabled: boolean): Promise<void> {
// ./tun2socks.exe \
// -tunName outline-tap0 -tunDNS 1.1.1.1,9.9.9.9 \
// -tunAddr 10.0.85.2 -tunGw 10.0.85.1 -tunMask 255.255.255.0 \
Expand All @@ -239,37 +244,42 @@ class GoTun2socks {
args.push('-dnsFallback');
}

this.autoRestart = true;
this.process.onStdErr = async (data?: string | Buffer) => {
if (!data?.toString().includes('tun2socks running')) {
return;
this.stopRequested = false;
let autoRestart = false;
do {
if (autoRestart) {
console.warn(`tun2socks exited unexpectedly. Restarting...`);
}
console.debug('tun2socks started');
this.process.onStdErr = null;
// try to auto restart tun2socks
const exitCode = await this.process.waitForEnd();
if (this.autoRestart) {
console.warn(`tun2socks exited unexpectedly with code/signal: ${exitCode}. Restarting...`);
this.start(isUdpEnabled);
} else {
console.info(`tun2socks exited with ${exitCode} as expected`);
autoRestart = false;
this.process.onStdErr = (data?: string | Buffer) => {
if (data?.toString().includes('tun2socks running')) {
console.debug('tun2socks started');
autoRestart = true;
this.process.onStdErr = null;
}
};
try {
await this.process.launch(args);
console.info('tun2socks exited with no errors');
} catch (e) {
console.error(`tun2socks terminated due to ${e}`);
}
};
this.process.launch(args);
} while (!this.stopRequested && autoRestart);
}

stop() {
this.autoRestart = false;
this.stopRequested = true;
return this.process.stop();
}

/**
* Checks connectivity and exits with an error code as defined in `errors.ErrorCode`
* Checks connectivity and exits with an error code as defined in `errors.ErrorCode`.
* If exit code is not zero, a `ProcessTerminatedExitCodeError` might be thrown.
* -tun* and -dnsFallback options have no effect on this mode.
*/
checkConnectivity() {
console.debug('using tun2socks to check connectivity');
this.process.launch([
return this.process.launch([
'-proxyHost',
this.config.host || '',
'-proxyPort',
Expand All @@ -280,7 +290,6 @@ class GoTun2socks {
this.config.method || '',
'-checkConnectivity',
]);
return this.process.waitForEnd();
}

enableDebugMode() {
Expand All @@ -293,18 +302,17 @@ class GoTun2socks {
// forwarding and validates the proxy credentials. Resolves with a boolean indicating whether UDP
// forwarding is supported. Throws if the checks fail or if the process fails to start.
async function checkConnectivity(config: ShadowsocksSessionConfig) {
let exitCode: number | string = errors.ErrorCode.UNEXPECTED;
try {
exitCode = await new GoTun2socks(config).checkConnectivity();
} catch (e) {
console.error(`connectivity check failed: ${e}`);
throw new errors.UnexpectedPluginError();
}
if (exitCode === errors.ErrorCode.NO_ERROR) {
await new GoTun2socks(config).checkConnectivity();
return true;
} else if (exitCode === errors.ErrorCode.UDP_RELAY_NOT_ENABLED) {
// Don't treat lack of UDP support as an error, relay to the caller.
return false;
} catch (e) {
console.error(`connectivity check error: ${e}`);
if (e instanceof ProcessTerminatedExitCodeError) {
if (e.exitCode === ErrorCode.UDP_RELAY_NOT_ENABLED) {
return false;
}
throw fromErrorCode(e.exitCode);
}
throw new UnexpectedPluginError();
}
throw errors.fromErrorCode(typeof exitCode === 'number' ? exitCode : errors.ErrorCode.UNEXPECTED);
}
109 changes: 60 additions & 49 deletions src/electron/process.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,24 @@ import {ChildProcess, spawn} from 'node:child_process';
import {basename} from 'node:path';
import process from 'node:process';

/**
* A child process is terminated abnormally, caused by a non-zero exit code.
*/
export class ProcessTerminatedExitCodeError extends Error {
constructor(public readonly exitCode: number) {
super(`Process terminated by non-zero exit code: ${exitCode}`);
}
}

/**
* A child process is terminated abnormally, caused by a signal string.
*/
export class ProcessTerminatedSignalError extends Error {
constructor(public readonly signal: string) {
super(`Process terminated by signal: ${signal}`);
}
}

// Simple "one shot" child process launcher.
//
// NOTE: Because there is no way in Node.js to tell whether a process launched successfully,
Expand All @@ -24,9 +42,13 @@ import process from 'node:process';
// found).
export class ChildProcessHelper {
private readonly processName: string;
private subProcess?: ChildProcess = null;
private exitCodePromise?: Promise<number | string> = Promise.resolve('not started');
private isDebug = false;
private childProcess?: ChildProcess = null;
private waitProcessToExit?: Promise<void>;

/**
* Whether to enable verbose logging for the process. Must be called before launch().
*/
public isDebugModeEnabled = false;

private stdErrListener?: (data?: string | Buffer) => void;

Expand All @@ -35,29 +57,38 @@ export class ChildProcessHelper {
}

/**
* Starts the process with the given args. If enableDebug() has been called, then the process is
* started in verbose mode if supported. This method will only start the process, it will not
* wait for it to be ended. Please use stop() or waitForExit() instead.
* Start the process with the given args and wait for the process to exit. If `isDebugModeEnabled`
* is `true`, the process is started in verbose mode if supported.
*
* If the process does not exist normally (i.e., exit code !== 0 or received a signal), it will
* throw either `ProcessTerminatedExitCodeError` or `ProcessTerminatedSignalError`.
*
* @param args The args for the process
*/
launch(args: string[]): void {
if (this.subProcess) {
async launch(args: string[]): Promise<void> {
if (this.childProcess) {
throw new Error(`subprocess ${this.processName} has already been launched`);
}
this.subProcess = spawn(this.path, args);
this.exitCodePromise = new Promise(resolve => {
this.childProcess = spawn(this.path, args);
return (this.waitProcessToExit = new Promise((resolve, reject) => {
const onExit = (code?: number, signal?: string) => {
if (this.subProcess) {
this.subProcess.removeAllListeners();
this.subProcess = null;
if (this.childProcess) {
this.childProcess.removeAllListeners();
this.childProcess = null;
} else {
// When listening to both the 'exit' and 'error' events, guard against accidentally
// invoking handler functions multiple times.
return;
}

logExit(this.processName, code, signal);
resolve(code ?? signal);
if (code === 0) {
resolve();
} else if (code) {
reject(new ProcessTerminatedExitCodeError(code));
} else {
reject(new ProcessTerminatedSignalError(signal));
}
};

const onStdErr = (data?: string | Buffer) => {
Expand All @@ -68,63 +99,43 @@ export class ChildProcessHelper {
this.stdErrListener(data);
}
};
this.subProcess.stderr.on('data', onStdErr.bind(this));
this.childProcess.stderr.on('data', onStdErr.bind(this));

if (this.isDebugModeEnabled) {
// Redirect subprocess output while bypassing the Node console. This makes sure we don't
// send web traffic information to Sentry.
this.subProcess.stdout.pipe(process.stdout);
this.subProcess.stderr.pipe(process.stderr);
this.childProcess.stdout.pipe(process.stdout);
this.childProcess.stderr.pipe(process.stderr);
}

// We have to listen for both events: error means the process could not be launched and in that
// case exit will not be invoked.
this.subProcess.on('error', onExit.bind(this));
this.subProcess.on('exit', onExit.bind(this));
});
this.childProcess.on('error', onExit.bind(this));
this.childProcess.on('exit', onExit.bind(this));
}));
}

/**
* Try to kill the process and wait for the exit code.
* @returns Either an exit code or a signal string (if the process is ended by a signal).
* Try to kill the process and wait for the process to exit.
*
* If the process does not exist normally (i.e., exit code !== 0 or received a signal), it will
* throw either `ProcessTerminatedExitCodeError` or `ProcessTerminatedSignalError`.
*/
stop(): Promise<number | string> {
if (!this.subProcess) {
stop(): Promise<void> {
if (!this.childProcess) {
// Never started.
return;
}
this.subProcess.kill();
return this.exitCodePromise;
}

/**
* Wait for the process to end, and get out the exit code.
* @returns Either an exit code or a signal string (if the process is ended by a signal).
*/
waitForEnd(): Promise<number | string> {
return this.exitCodePromise;
this.childProcess.kill();
return this.waitProcessToExit;
}

set onStdErr(listener: ((data?: string | Buffer) => void) | undefined) {
this.stdErrListener = listener;
if (!this.stdErrListener && !this.isDebugModeEnabled) {
this.subProcess?.stderr.removeAllListeners();
this.childProcess?.stderr.removeAllListeners();
}
}

/**
* Whether to enable verbose logging for the process. Must be called before launch().
*/
set isDebugModeEnabled(value: boolean) {
this.isDebug = value;
}

/**
* Get a value indicates whether verbose logging for the process is enabled.
*/
get isDebugModeEnabled(): boolean {
return this.isDebug;
}
}

function logExit(processName: string, exitCode?: number, signal?: string) {
Expand Down

0 comments on commit 4dc8ff3

Please sign in to comment.