Skip to content

Commit 69f0788

Browse files
Fix hanging commands
In case the command is not related to LiveSync, once it finishes its work, CLI process should end. However, this works only on Windows, as the broker child process there is really detached from the parent. On macOS and Linux, the 'ipc' channel between the broker process and CLI, keeps both alive. So use the 'dispose' method, which is called at the end of command execution and disconnect the broker process. In case we are in LiveSync case, set shouldDispose to false and this way the broker will be kept alive. Once Ctrl + C is used to end the CLI process, send the finish message to broker and it will disconnect itself from CLI.
1 parent df53c2a commit 69f0788

9 files changed

+30
-10
lines changed

lib/commands/run.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ export class RunCommandBase implements ICommand {
55

66
public platform: string;
77
constructor(protected $platformService: IPlatformService,
8-
protected $liveSyncService: ILiveSyncService,
98
protected $projectData: IProjectData,
109
protected $options: IOptions,
1110
protected $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,

lib/services/analytics/analytics-broker-process.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,19 @@ const finishTracking = async (data?: ITrackingInformation) => {
2525
sentFinishMsg = true;
2626

2727
data = data || { type: TrackingTypes.Finish };
28+
const action = async () => {
29+
await sendDataForTracking(data);
30+
process.disconnect();
31+
};
2832

2933
if (receivedFinishMsg) {
30-
await sendDataForTracking(data);
34+
await action();
3135
} else {
3236
// In case we've got here without receiving "finish" message from parent (receivedFinishMsg is false)
3337
// there might be various reasons, but most probably the parent is dead.
3438
// However, there's no guarantee that we've received all messages. So wait some time before sending finish message to children.
3539
setTimeout(async () => {
36-
await sendDataForTracking(data);
40+
await action();
3741
}, 1000);
3842
}
3943
}

lib/services/analytics/analytics-service.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import { cache } from "../../common/decorators";
55
import { isInteractive } from '../../common/helpers';
66
import { DeviceTypes, AnalyticsClients } from "../../common/constants";
77

8-
export class AnalyticsService extends AnalyticsServiceBase implements IAnalyticsService {
8+
export class AnalyticsService extends AnalyticsServiceBase {
99
private static ANALYTICS_BROKER_START_TIMEOUT = 30 * 1000;
10+
private brokerProcess: ChildProcess;
1011

1112
constructor(protected $logger: ILogger,
1213
protected $options: IOptions,
@@ -56,7 +57,7 @@ export class AnalyticsService extends AnalyticsServiceBase implements IAnalytics
5657
gaSettings.customDimensions = gaSettings.customDimensions || {};
5758
gaSettings.customDimensions[GoogleAnalyticsCustomDimensions.client] = this.$options.analyticsClient || (isInteractive() ? AnalyticsClients.Cli : AnalyticsClients.Unknown);
5859

59-
const googleAnalyticsData: IGoogleAnalyticsTrackingInformation = _.merge({ type: TrackingTypes.GoogleAnalyticsData }, gaSettings, { category: AnalyticsClients.Cli });
60+
const googleAnalyticsData: IGoogleAnalyticsTrackingInformation = _.merge({ type: TrackingTypes.GoogleAnalyticsData, category: AnalyticsClients.Cli }, gaSettings);
6061
return this.sendMessageToBroker(googleAnalyticsData);
6162
}
6263
}
@@ -69,9 +70,9 @@ export class AnalyticsService extends AnalyticsServiceBase implements IAnalytics
6970
let label: string = "";
7071
label = this.addDataToLabel(label, platform);
7172

73+
// In some cases (like in case action is Build and platform is Android), we do not know if the deviceType is emulator or device.
74+
// Just exclude the device_type in this case.
7275
if (isForDevice !== null) {
73-
// In case action is Build and platform is Android, we do not know if the deviceType is emulator or device.
74-
// Just exclude the device_type in this case.
7576
const deviceType = isForDevice ? DeviceTypes.Device : (this.$mobileHelper.isAndroidPlatform(platform) ? DeviceTypes.Emulator : DeviceTypes.Simulator);
7677
label = this.addDataToLabel(label, deviceType);
7778
}
@@ -102,6 +103,12 @@ export class AnalyticsService extends AnalyticsServiceBase implements IAnalytics
102103
await this.trackInGoogleAnalytics(googleAnalyticsEventData);
103104
}
104105

106+
public dispose(): void {
107+
if (this.brokerProcess && this.shouldDisposeInstance) {
108+
this.brokerProcess.disconnect();
109+
}
110+
}
111+
105112
private addDataToLabel(label: string, newData: string): string {
106113
if (newData && label) {
107114
return `${label}_${newData}`;
@@ -156,6 +163,8 @@ export class AnalyticsService extends AnalyticsServiceBase implements IAnalytics
156163
});
157164
});
158165

166+
this.brokerProcess = broker;
167+
159168
resolve(broker);
160169
}
161170
}

lib/services/analytics/eqatec-analytics-provider.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ export class EqatecAnalyticsProvider extends AnalyticsServiceBase implements IAn
5151
this.tryStopEqatecMonitors();
5252
}
5353

54+
public dispose(): void {
55+
// Intentionally left blank.
56+
}
57+
5458
}
5559

5660
$injector.register("eqatecAnalyticsProvider", EqatecAnalyticsProvider);

lib/services/analytics/google-analytics-provider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export class GoogleAnalyticsProvider implements IGoogleAnalyticsProvider {
109109

110110
private getMacOSReleaseVersion(osRelease: string): string {
111111
// https://en.wikipedia.org/wiki/Darwin_(operating_system)#Release_history
112-
// Each macOS version is labeled 10.<version>, where it looks like <versions> is taken from the major version returned by os.release() (16.x.x for example) and substituting 4 from it.
112+
// Each macOS version is labeled 10.<version>, where it looks like <versions> is taken from the major version returned by os.release() (16.x.x for example) and subtracting 4 from it.
113113
// So the version becomes "10.12" in this case.
114114
// Could be improved by spawning `system_profiler SPSoftwareDataType` and getting the System Version line from the result.
115115
const majorVersion = osRelease && _.first(osRelease.split("."));

lib/services/livesync/livesync-command-helper.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ export class LiveSyncCommandHelper implements ILiveSyncCommandHelper {
77
private $iosDeviceOperations: IIOSDeviceOperations,
88
private $mobileHelper: Mobile.IMobileHelper,
99
private $platformsData: IPlatformsData,
10+
private $analyticsService: IAnalyticsService,
1011
private $errors: IErrors) {
12+
this.$analyticsService.setShouldDispose(this.$options.justlaunch || !this.$options.watch);
1113
}
1214

1315
public getPlatformsForOperation(platform: string): string[] {

lib/services/test-execution-service.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ class TestExecutionService implements ITestExecutionService {
2626
private $errors: IErrors,
2727
private $debugService: IDebugService,
2828
private $devicesService: Mobile.IDevicesService,
29+
private $analyticsService: IAnalyticsService,
2930
private $childProcess: IChildProcess) {
31+
this.$analyticsService.setShouldDispose(this.$options.justlaunch || !this.$options.watch);
3032
}
3133

3234
public platform: string;

npm-shrinkwrap.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)