Merge pull request #4533 from NativeScript/vladimirov/fix-ctrl-c

feat: introduce better way for handling Ctrl+C

Author: rosen-vladimirov

PublicAPI.md

@@ -70,6 +70,8 @@ const tns = require("nativescript");
 	* [deviceLog](#devicelog)
 * [previewQrCodeService](#previewqrcodeservice)
 	* [getPlaygroundAppQrCode](#getplaygroundappqrcode)
+* [cleanupService](#cleanupservice)
+	* [setCleanupLogFile](#setcleanuplogfile)
 
 ## Module projectService
 
@@ -1487,6 +1489,31 @@ tns.previewQrCodeService.getPlaygroundAppQrCode()
 	});
 ```
 
+## cleanupService
+The `cleanupService` is used to handle actions that should be executed after CLI's process had exited. This is an internal service, that runs detached childProcess in which it executes CLI's cleanup actions once CLI is dead. As the process is detached, logs from it are not shown anywhere, so the service exposes a way to add log file in which the child process will write its logs.
+
+### setCleanupLogFile
+Defines the log file location where the child cleanup process will write its logs.
+
+> NOTE: You must call this method immediately after requiring NativeScript CLI. In case you call it after the cleanup process had started, it will not use the passed log file.
+
+* Definition
+```TypeScript
+/**
+ * Sets the file in which the cleanup process will write its logs.
+ * This method must be called before starting the cleanup process, i.e. when CLI is initialized.
+ * @param {string} filePath Path to file where the logs will be written. The logs are appended to the passed file.
+ * @returns {void}
+ */
+setCleanupLogFile(filePath: string): void;
+```
+
+* Usage
+```JavaScript
+const tns = require("nativescript");
+tns.cleanupService.setCleanupLogFile("/Users/username/cleanup-logs.txt");
+```
+
 ## How to add a new method to Public API
 CLI is designed as command line tool and when it is used as a library, it does not give you access to all of the methods. This is mainly implementation detail. Most of the CLI's code is created to work in command line, not as a library, so before adding method to public API, most probably it will require some modification.
 For example the `$options` injected module contains information about all `--` options passed on the terminal. When the CLI is used as a library, the options are not populated. Before adding method to public API, make sure its implementation does not rely on `$options`.

lib/bootstrap.ts

@@ -192,3 +192,4 @@ $injector.require("qrCodeTerminalService", "./services/qr-code-terminal-service"
 $injector.require("testInitializationService", "./services/test-initialization-service");
 
 $injector.require("networkConnectivityValidator", "./helpers/network-connectivity-validator");
+$injector.requirePublic("cleanupService", "./services/cleanup-service");

lib/commands/debug.ts

@@ -91,14 +91,16 @@ export class DebugIOSCommand implements ICommand {
 		private $sysInfo: ISysInfo,
 		private $projectData: IProjectData,
 		$iosDeviceOperations: IIOSDeviceOperations,
-		$iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider) {
+		$iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider,
+		$cleanupService: ICleanupService) {
 		this.$projectData.initializeProjectData();
 		// Do not dispose ios-device-lib, so the process will remain alive and the debug application (NativeScript Inspector or Chrome DevTools) will be able to connect to the socket.
 		// In case we dispose ios-device-lib, the socket will be closed and the code will fail when the debug application tries to read/send data to device socket.
 		// That's why the `$ tns debug ios --justlaunch` command will not release the terminal.
 		// In case we do not set it to false, the dispose will be called once the command finishes its execution, which will prevent the debugging.
 		$iosDeviceOperations.setShouldDispose(false);
 		$iOSSimulatorLogProvider.setShouldDispose(false);
+		$cleanupService.setShouldDispose(false);
 	}
 
 	public execute(args: string[]): Promise<void> {

lib/commands/preview.ts

@@ -13,8 +13,10 @@ export class PreviewCommand implements ICommand {
 		private $projectData: IProjectData,
 		private $options: IOptions,
 		private $previewAppLogProvider: IPreviewAppLogProvider,
-		private $previewQrCodeService: IPreviewQrCodeService) {
-			this.$analyticsService.setShouldDispose(this.$options.justlaunch || !this.$options.watch);
+		private $previewQrCodeService: IPreviewQrCodeService,
+		$cleanupService: ICleanupService) {
+			this.$analyticsService.setShouldDispose(false);
+			$cleanupService.setShouldDispose(false);
 		}
 
 	public async execute(): Promise<void> {

lib/commands/test.ts

@@ -10,6 +10,7 @@ abstract class TestCommandBase {
 	protected abstract $options: IOptions;
 	protected abstract $platformEnvironmentRequirements: IPlatformEnvironmentRequirements;
 	protected abstract $errors: IErrors;
+	protected abstract $cleanupService: ICleanupService;
 
 	async execute(args: string[]): Promise<void> {
 		await this.$testExecutionService.startKarmaServer(this.platform, this.$projectData, this.projectFilesConfig);
@@ -18,6 +19,7 @@ abstract class TestCommandBase {
 	async canExecute(args: string[]): Promise<boolean | ICanExecuteCommandOutput> {
 		this.$projectData.initializeProjectData();
 		this.$analyticsService.setShouldDispose(this.$options.justlaunch || !this.$options.watch);
+		this.$cleanupService.setShouldDispose(this.$options.justlaunch || !this.$options.watch);
 		this.projectFilesConfig = helpers.getProjectFilesConfig({ isReleaseBuild: this.$options.release });
 
 		const output = await this.$platformEnvironmentRequirements.checkEnvironmentRequirements({
@@ -51,7 +53,8 @@ class TestAndroidCommand extends TestCommandBase implements ICommand {
 		protected $analyticsService: IAnalyticsService,
 		protected $options: IOptions,
 		protected $platformEnvironmentRequirements: IPlatformEnvironmentRequirements,
-		protected $errors: IErrors) {
+		protected $errors: IErrors,
+		protected $cleanupService: ICleanupService) {
 		super();
 	}
 
@@ -65,7 +68,8 @@ class TestIosCommand extends TestCommandBase implements ICommand {
 		protected $analyticsService: IAnalyticsService,
 		protected $options: IOptions,
 		protected $platformEnvironmentRequirements: IPlatformEnvironmentRequirements,
-		protected $errors: IErrors) {
+		protected $errors: IErrors,
+		protected $cleanupService: ICleanupService) {
 		super();
 	}
 

lib/common/bootstrap.ts

@@ -6,7 +6,6 @@ $injector.require("errors", "./errors");
 $injector.requirePublic("fs", "./file-system");
 $injector.require("hostInfo", "./host-info");
 $injector.require("osInfo", "./os-info");
-$injector.require("timers", "./timers");
 
 $injector.require("dispatcher", "./dispatchers");
 $injector.require("commandDispatcher", "./dispatchers");
@@ -85,7 +84,6 @@ $injector.require("iOSEmulatorServices", "./mobile/ios/simulator/ios-emulator-se
 $injector.require("wp8EmulatorServices", "./mobile/wp8/wp8-emulator-services");
 
 $injector.require("autoCompletionService", "./services/auto-completion-service");
-$injector.require("processService", "./services/process-service");
 $injector.requirePublic("settingsService", "./services/settings-service");
 $injector.require("opener", "./opener");
 $injector.require("microTemplateService", "./services/micro-templating-service");

lib/common/child-process.ts

@@ -62,7 +62,15 @@ export class ChildProcess extends EventEmitter implements IChildProcess {
 			let isResolved = false;
 			let capturedOut = "";
 			let capturedErr = "";
-
+			let killTimer: NodeJS.Timer = null;
+
+			if (spawnFromEventOptions && spawnFromEventOptions.timeout) {
+				this.$logger.trace(`Setting maximum time for execution of current child process to ${spawnFromEventOptions.timeout}`);
+				killTimer = setTimeout(() => {
+					this.$logger.trace(`Sending SIGTERM to current child process as maximum time for execution ${spawnFromEventOptions.timeout} had passed.`);
+					childProcess.kill('SIGTERM');
+				}, spawnFromEventOptions.timeout);
+			}
 			if (childProcess.stdout) {
 				childProcess.stdout.on("data", (data: string) => {
 					if (spawnFromEventOptions && spawnFromEventOptions.emitOptions && spawnFromEventOptions.emitOptions.eventName) {
@@ -91,17 +99,27 @@ export class ChildProcess extends EventEmitter implements IChildProcess {
 					exitCode: exitCode
 				};
 
+				const clearKillTimer = () => {
+					if (killTimer) {
+						clearTimeout(killTimer);
+					}
+				};
+
+				const resolveAction = () => {
+					isResolved = true;
+					resolve(result);
+					clearKillTimer();
+				};
+
 				if (spawnFromEventOptions && spawnFromEventOptions.throwError === false) {
 					if (!isResolved) {
 						this.$logger.trace("Result when throw error is false:");
 						this.$logger.trace(result);
-						isResolved = true;
-						resolve(result);
+						resolveAction();
 					}
 				} else {
 					if (exitCode === 0) {
-						isResolved = true;
-						resolve(result);
+						resolveAction();
 					} else {
 						let errorMessage = `Command ${command} failed with exit code ${exitCode}`;
 						if (capturedErr) {
@@ -111,6 +129,7 @@ export class ChildProcess extends EventEmitter implements IChildProcess {
 						if (!isResolved) {
 							isResolved = true;
 							reject(new Error(errorMessage));
+							clearKillTimer();
 						}
 					}
 				}

lib/common/commands/device/device-log-stream.ts

@@ -7,8 +7,10 @@ export class OpenDeviceLogStreamCommand implements ICommand {
 		private $options: IOptions,
 		private $deviceLogProvider: Mobile.IDeviceLogProvider,
 		private $loggingLevels: Mobile.ILoggingLevels,
-		$iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider) {
+		$iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider,
+		$cleanupService: ICleanupService) {
 			$iOSSimulatorLogProvider.setShouldDispose(false);
+			$cleanupService.setShouldDispose(false);
 		}
 
 	allowedParameters: ICommandParameter[] = [];

lib/common/declarations.d.ts

@@ -207,11 +207,6 @@ declare const enum TrackingTypes {
 	 * Defines that the broker process should get and track the data from preview app to Google Analytics
 	 */
 	PreviewAppData = "PreviewAppData",
-
-	/**
-	 * Defines that all information has been sent and no more data will be tracked in current session.
-	 */
-	Finish = "finish"
 }
 
 /**
@@ -653,7 +648,8 @@ interface ISpawnFromEventOptions {
 	throwError: boolean;
 	emitOptions?: {
 		eventName: string;
-	}
+	},
+	timeout?: number;
 }
 
 interface IProjectDir {
@@ -1471,11 +1467,6 @@ interface INet {
 	waitForPortToListen(waitForPortListenData: IWaitForPortListenData): Promise<boolean>;
 }
 
-interface IProcessService {
-	listenersCount: number;
-	attachToProcessExitSignals(context: any, callback: () => void): void;
-}
-
 interface IDependencyInformation {
 	name: string;
 	version?: string;

lib/common/definitions/timers.d.ts

@@ -15,19 +15,11 @@ export class HttpClient implements Server.IHttpClient {
 	// We receive multiple response packets every ms but we don't need to be very aggressive here.
 	private static STUCK_RESPONSE_CHECK_INTERVAL = 10000;
 
-	private defaultUserAgent: string;
-	private cleanupData: ICleanupRequestData[];
+private defaultUserAgent: string;
 
 	constructor(private $logger: ILogger,
-		private $processService: IProcessService,
 		private $proxyService: IProxyService,
 		private $staticConfig: Config.IStaticConfig) {
-		this.cleanupData = [];
-		this.$processService.attachToProcessExitSignals(this, () => {
-			this.cleanupData.forEach(d => {
-				this.cleanupAfterRequest(d);
-			});
-		});
 	}
 
 	public async httpRequest(options: any, proxySettings?: IProxySettings): Promise<Server.IResponse> {
@@ -107,7 +99,6 @@ export class HttpClient implements Server.IHttpClient {
 		const result = new Promise<Server.IResponse>((resolve, reject) => {
 			let timerId: NodeJS.Timer;
 			const cleanupRequestData: ICleanupRequestData = Object.create({ timers: [] });
-			this.cleanupData.push(cleanupRequestData);
 
 			const promiseActions: IPromiseActions<Server.IResponse> = {
 				resolve,

lib/common/http-client.ts

@@ -16,7 +16,6 @@ export class LogcatHelper implements Mobile.ILogcatHelper {
 		private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
 		private $logger: ILogger,
 		private $injector: IInjector,
-		private $processService: IProcessService,
 		private $devicesService: Mobile.IDevicesService) {
 		this.mapDevicesLoggingData = Object.create(null);
 	}
@@ -53,8 +52,6 @@ export class LogcatHelper implements Mobile.ILogcatHelper {
 				const lineText = line.toString();
 				this.$deviceLogProvider.logData(lineText, this.$devicePlatformsConstants.Android, deviceIdentifier);
 			});
-
-			this.$processService.attachToProcessExitSignals(this, logcatStream.kill);
 		}
 	}
 
@@ -72,8 +69,6 @@ export class LogcatHelper implements Mobile.ILogcatHelper {
 			logcatDumpStream.removeAllListeners();
 			lineStream.removeAllListeners();
 		});
-
-		this.$processService.attachToProcessExitSignals(this, logcatDumpStream.kill);
 	}
 
 	/**

lib/common/mobile/android/logcat-helper.ts

@@ -9,16 +9,11 @@ export class IOSDeviceOperations extends EventEmitter implements IIOSDeviceOpera
 	public shouldDispose: boolean;
 	private deviceLib: IOSDeviceLib.IOSDeviceLib;
 
-	constructor(private $logger: ILogger,
-		private $processService: IProcessService) {
+	constructor(private $logger: ILogger) {
 		super();
 
 		this.isInitialized = false;
 		this.shouldDispose = true;
-		this.$processService.attachToProcessExitSignals(this, () => {
-			this.setShouldDispose(true);
-			this.dispose();
-		});
 	}
 
 	public async install(ipaPath: string, deviceIdentifiers: string[], errorHandler: DeviceOperationErrorHandler): Promise<IOSDeviceResponse> {

lib/common/mobile/ios/device/ios-device-operations.ts

@@ -20,7 +20,6 @@ export class IOSDevice extends IOSDeviceBase {
 		protected $deviceLogProvider: Mobile.IDeviceLogProvider,
 		protected $lockService: ILockService,
 		private $iOSSocketRequestExecutor: IiOSSocketRequestExecutor,
-		protected $processService: IProcessService,
 		private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
 		private $iOSDeviceProductNameMapper: Mobile.IiOSDeviceProductNameMapper,
 		private $iosDeviceOperations: IIOSDeviceOperations,

lib/common/mobile/ios/device/ios-device.ts

@@ -6,7 +6,6 @@ export abstract class IOSDeviceBase implements Mobile.IiOSDevice {
 	protected abstract $errors: IErrors;
 	protected abstract $deviceLogProvider: Mobile.IDeviceLogProvider;
 	protected abstract $iOSDebuggerPortService: IIOSDebuggerPortService;
-	protected abstract $processService: IProcessService;
 	protected abstract $lockService: ILockService;
 	abstract deviceInfo: Mobile.IDeviceInfo;
 	abstract applicationManager: Mobile.IDeviceApplicationManager;
@@ -33,8 +32,6 @@ export abstract class IOSDeviceBase implements Mobile.IiOSDevice {
 					this.cachedSockets[appId].on("close", async () => {
 						await this.destroyDebugSocket(appId);
 					});
-
-					this.$processService.attachToProcessExitSignals(this, () => this.destroyDebugSocket(appId));
 				}
 
 				return this.cachedSockets[appId];

lib/common/mobile/ios/ios-device-base.ts

@@ -15,18 +15,10 @@ export class IOSSimulatorApplicationManager extends ApplicationManagerBase {
 		private device: Mobile.IiOSDevice,
 		private $options: IOptions,
 		private $fs: IFileSystem,
-		private $processService: IProcessService,
 		private $deviceLogProvider: Mobile.IDeviceLogProvider,
 		$logger: ILogger,
 		$hooksService: IHooksService) {
 		super($logger, $hooksService);
-		this.$processService.attachToProcessExitSignals(this, () => {
-			for (const appId in this._lldbProcesses) {
-				/* tslint:disable:no-floating-promises */
-				this.detachNativeDebugger(appId);
-				/* tslint:enable:no-floating-promises */
-			}
-		});
 	}
 
 	public async getInstalledApplications(): Promise<string[]> {

lib/common/mobile/ios/simulator/ios-simulator-application-manager.ts

@@ -22,8 +22,7 @@ export class IOSSimulator extends IOSDeviceBase implements Mobile.IiOSDevice {
 		private $iOSEmulatorServices: Mobile.IiOSSimulatorService,
 		private $iOSNotification: IiOSNotification,
 		private $iOSSimulatorLogProvider: Mobile.IiOSSimulatorLogProvider,
-		private $logger: ILogger,
-		protected $processService: IProcessService) {
+		private $logger: ILogger) {
 		super();
 		this.applicationManager = this.$injector.resolve(applicationManagerPath.IOSSimulatorApplicationManager, { iosSim: this.$iOSSimResolver.iOSSim, device: this });
 		this.fileSystem = this.$injector.resolve(fileSystemPath.IOSSimulatorFileSystem, { iosSim: this.$iOSSimResolver.iOSSim });

lib/common/mobile/ios/simulator/ios-simulator-device.ts

@@ -8,7 +8,6 @@ export class IOSSimulatorLogProvider extends EventEmitter implements Mobile.IiOS
 
 	constructor(private $iOSSimResolver: Mobile.IiOSSimResolver,
 		private $logger: ILogger,
-		private $processService: IProcessService,
 		private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
 		private $deviceLogProvider: Mobile.IDeviceLogProvider) {
 			super();
@@ -47,8 +46,6 @@ export class IOSSimulatorLogProvider extends EventEmitter implements Mobile.IiOS
 				deviceLogChildProcess.stderr.on("data", action.bind(this));
 			}
 
-			this.$processService.attachToProcessExitSignals(this, deviceLogChildProcess.kill);
-
 			this.simulatorsLoggingEnabled[deviceId] = true;
 			this.simulatorsLogProcess[deviceId] = deviceLogChildProcess;
 		}