Skip to content

feat: introduce better way for handling Ctrl+C #4533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Apr 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions PublicAPI.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ const tns = require("nativescript");
* [deviceLog](#devicelog)
* [previewQrCodeService](#previewqrcodeservice)
* [getPlaygroundAppQrCode](#getplaygroundappqrcode)
* [cleanupService](#cleanupservice)
* [setCleanupLogFile](#setcleanuplogfile)

## Module projectService

Expand Down Expand Up @@ -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`.
Expand Down
1 change: 1 addition & 0 deletions lib/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
4 changes: 3 additions & 1 deletion lib/commands/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
6 changes: 4 additions & 2 deletions lib/commands/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
8 changes: 6 additions & 2 deletions lib/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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({
Expand Down Expand Up @@ -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();
}

Expand All @@ -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();
}

Expand Down
2 changes: 0 additions & 2 deletions lib/common/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down
29 changes: 24 additions & 5 deletions lib/common/child-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -111,6 +129,7 @@ export class ChildProcess extends EventEmitter implements IChildProcess {
if (!isResolved) {
isResolved = true;
reject(new Error(errorMessage));
clearKillTimer();
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion lib/common/commands/device/device-log-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand Down
13 changes: 2 additions & 11 deletions lib/common/declarations.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}

/**
Expand Down Expand Up @@ -653,7 +648,8 @@ interface ISpawnFromEventOptions {
throwError: boolean;
emitOptions?: {
eventName: string;
}
},
timeout?: number;
}

interface IProjectDir {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions lib/common/definitions/timers.d.ts

This file was deleted.

11 changes: 1 addition & 10 deletions lib/common/http-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 0 additions & 5 deletions lib/common/mobile/android/logcat-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -72,8 +69,6 @@ export class LogcatHelper implements Mobile.ILogcatHelper {
logcatDumpStream.removeAllListeners();
lineStream.removeAllListeners();
});

this.$processService.attachToProcessExitSignals(this, logcatDumpStream.kill);
}

/**
Expand Down
7 changes: 1 addition & 6 deletions lib/common/mobile/ios/device/ios-device-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
1 change: 0 additions & 1 deletion lib/common/mobile/ios/device/ios-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions lib/common/mobile/ios/ios-device-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[]> {
Expand Down
3 changes: 1 addition & 2 deletions lib/common/mobile/ios/simulator/ios-simulator-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
3 changes: 0 additions & 3 deletions lib/common/mobile/ios/simulator/ios-simulator-log-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down
Loading