Skip to content

Commit e18a8a7

Browse files
miroslavaivanovamiroslavaivanova
authored
Merge pull request #5098 from NativeScript/tachev/preview-recovery
feat: reduce HMR crashes in Preview and recover properly after them
2 parents 9a1f37f + 373593a commit e18a8a7

File tree

7 files changed

+102
-42
lines changed

7 files changed

+102
-42
lines changed

‎lib/controllers/preview-app-controller.ts‎

Lines changed: 91 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import { Device, FilesPayload } from "nativescript-preview-sdk";
21
import { TrackActionNames, PREPARE_READY_EVENT_NAME } from "../constants";
32
import { PrepareController } from "./prepare-controller";
3+
import { Device, FilesPayload } from "nativescript-preview-sdk";
44
import { performanceLog } from "../common/decorators";
5-
import { stringify } from "../common/helpers";
5+
import { stringify, deferPromise } from "../common/helpers";
66
import { HmrConstants } from "../common/constants";
77
import { EventEmitter } from "events";
88
import { PrepareDataService } from "../services/prepare-data-service";
@@ -11,7 +11,14 @@ import { PreviewAppLiveSyncEvents } from "../services/livesync/playground/previe
1111
export class PreviewAppController extends EventEmitter implements IPreviewAppController {
1212
private prepareReadyEventHandler: any = null;
1313
private deviceInitializationPromise: IDictionary<boolean> = {};
14-
private promise = Promise.resolve();
14+
private devicesLiveSyncChain: IDictionary<Promise<void>> = {};
15+
private devicesCanExecuteHmr: IDictionary<boolean> = {};
16+
// holds HMR files per device in order to execute batch upload on fast updates
17+
private devicesHmrFiles: IDictionary<string[]> = {};
18+
// holds app files per device in order to execute batch upload on fast updates on failed HMR or --no-hmr
19+
private devicesAppFiles: IDictionary<string[]> = {};
20+
// holds the current HMR hash per device in order to watch the proper hash status on fast updates
21+
private devicesCurrentHmrHash: IDictionary<string> = {};
1522

1623
constructor(
1724
private $analyticsService: IAnalyticsService,
@@ -89,6 +96,7 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon
8996

9097
if (data.useHotModuleReload) {
9198
this.$hmrStatusService.attachToHmrStatusEvent();
99+
this.devicesCanExecuteHmr[device.id] = true;
92100
}
93101

94102
await this.$previewAppPluginsService.comparePluginsOnDevice(data, device);
@@ -109,13 +117,13 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon
109117
await this.$prepareController.prepare(prepareData);
110118

111119
try {
112-
const payloads = await this.getInitialFilesForPlatformSafe(data, device.platform);
120+
const payloads = await this.getInitialFilesForDeviceSafe(data, device);
113121
return payloads;
114122
} finally {
115123
this.deviceInitializationPromise[device.id] = null;
116124
}
117125
} catch (error) {
118-
this.$logger.trace(`Error while sending files on device ${device && device.id}. Error is`, error);
126+
this.$logger.trace(`Error while sending files on device '${device && device.id}'. Error is`, error);
119127
this.emit(PreviewAppLiveSyncEvents.PREVIEW_APP_LIVE_SYNC_ERROR, {
120128
error,
121129
data,
@@ -129,52 +137,101 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon
129137

130138
@performanceLog()
131139
private async handlePrepareReadyEvent(data: IPreviewAppLiveSyncData, currentPrepareData: IFilesChangeEventData) {
132-
await this.promise
133-
.then(async () => {
134-
const { hmrData, files, platform } = currentPrepareData;
135-
const platformHmrData = _.cloneDeep(hmrData);
136-
137-
this.promise = this.syncFilesForPlatformSafe(data, { filesToSync: files }, platform);
138-
await this.promise;
139-
140-
if (data.useHotModuleReload && platformHmrData.hash) {
141-
const devices = this.$previewDevicesService.getDevicesForPlatform(platform);
142-
143-
await Promise.all(_.map(devices, async (previewDevice: Device) => {
144-
const status = await this.$hmrStatusService.getHmrStatus(previewDevice.id, platformHmrData.hash);
145-
if (status === HmrConstants.HMR_ERROR_STATUS) {
146-
const originalUseHotModuleReload = data.useHotModuleReload;
147-
data.useHotModuleReload = false;
148-
await this.syncFilesForPlatformSafe(data, { filesToSync: platformHmrData.fallbackFiles }, platform, previewDevice.id);
149-
data.useHotModuleReload = originalUseHotModuleReload;
150-
}
151-
}));
140+
const { hmrData, files, platform } = currentPrepareData;
141+
const platformHmrData = _.cloneDeep(hmrData);
142+
const connectedDevices = this.$previewDevicesService.getDevicesForPlatform(platform);
143+
if (!connectedDevices || !connectedDevices.length) {
144+
this.$logger.warn(`Unable to find any connected devices for platform '${platform}'. In order to execute live sync, open your Preview app and optionally re-scan the QR code using the Playground app.`);
145+
return;
146+
}
147+
148+
await Promise.all(_.map(connectedDevices, async (device) => {
149+
const previousSync = this.devicesLiveSyncChain[device.id] || Promise.resolve();
150+
const currentSyncDeferPromise = deferPromise<void>();
151+
this.devicesLiveSyncChain[device.id] = currentSyncDeferPromise.promise;
152+
this.devicesCurrentHmrHash[device.id] = this.devicesCurrentHmrHash[device.id] || platformHmrData.hash;
153+
if (data.useHotModuleReload) {
154+
this.devicesHmrFiles[device.id] = this.devicesHmrFiles[device.id] || [];
155+
this.devicesHmrFiles[device.id].push(...files);
156+
this.devicesAppFiles[device.id] = platformHmrData.fallbackFiles;
157+
} else {
158+
this.devicesHmrFiles[device.id] = [];
159+
this.devicesAppFiles[device.id] = files;
160+
}
161+
162+
await previousSync;
163+
164+
try {
165+
let canExecuteHmrSync = false;
166+
const hmrHash = this.devicesCurrentHmrHash[device.id];
167+
this.devicesCurrentHmrHash[device.id] = null;
168+
if (data.useHotModuleReload && hmrHash) {
169+
if (this.devicesCanExecuteHmr[device.id]) {
170+
canExecuteHmrSync = true;
171+
}
152172
}
153-
});
173+
174+
const filesToSync = canExecuteHmrSync ? this.devicesHmrFiles[device.id] : this.devicesAppFiles[device.id];
175+
if (!filesToSync || !filesToSync.length) {
176+
this.$logger.info(`Skipping files sync for device ${this.getDeviceDisplayName(device)}. The changes are already batch transferred in a previous sync.`);
177+
currentSyncDeferPromise.resolve();
178+
return;
179+
}
180+
181+
this.devicesHmrFiles[device.id] = [];
182+
this.devicesAppFiles[device.id] = [];
183+
if (canExecuteHmrSync) {
184+
this.$hmrStatusService.watchHmrStatus(device.id, hmrHash);
185+
await this.syncFilesForPlatformSafe(data, { filesToSync }, platform, device);
186+
const status = await this.$hmrStatusService.getHmrStatus(device.id, hmrHash);
187+
if (!status) {
188+
this.devicesCanExecuteHmr[device.id] = false;
189+
this.$logger.warn(`Unable to get LiveSync status from the Preview app for device ${this.getDeviceDisplayName(device)}. Ensure the app is running in order to sync changes.`);
190+
} else {
191+
this.devicesCanExecuteHmr[device.id] = status === HmrConstants.HMR_SUCCESS_STATUS;
192+
}
193+
} else {
194+
const noHmrData = _.assign({}, data, { useHotModuleReload: false });
195+
await this.syncFilesForPlatformSafe(noHmrData, { filesToSync }, platform, device);
196+
this.devicesCanExecuteHmr[device.id] = true;
197+
}
198+
currentSyncDeferPromise.resolve();
199+
} catch (e) {
200+
currentSyncDeferPromise.resolve();
201+
}
202+
}));
203+
}
204+
205+
private getDeviceDisplayName(device: Device) {
206+
return `${device.name} (${device.id})`.cyan;
154207
}
155208

156-
private async getInitialFilesForPlatformSafe(data: IPreviewAppLiveSyncData, platform: string): Promise<FilesPayload> {
157-
this.$logger.info(`Start sending initial files for platform ${platform}.`);
209+
private async getInitialFilesForDeviceSafe(data: IPreviewAppLiveSyncData, device: Device): Promise<FilesPayload> {
210+
const platform = device.platform;
211+
this.$logger.info(`Start sending initial files for device ${this.getDeviceDisplayName(device)}.`);
158212

159213
try {
160214
const payloads = this.$previewAppFilesService.getInitialFilesPayload(data, platform);
161-
this.$logger.info(`Successfully sent initial files for platform ${platform}.`);
215+
this.$logger.info(`Successfully sent initial files for device ${this.getDeviceDisplayName(device)}.`);
162216
return payloads;
163217
} catch (err) {
164-
this.$logger.warn(`Unable to apply changes for platform ${platform}. Error is: ${err}, ${stringify(err)}`);
218+
this.$logger.warn(`Unable to apply changes for device ${this.getDeviceDisplayName(device)}. Error is: ${err}, ${stringify(err)}`);
165219
}
166220
}
167221

168-
private async syncFilesForPlatformSafe(data: IPreviewAppLiveSyncData, filesData: IPreviewAppFilesData, platform: string, deviceId?: string): Promise<void> {
222+
private async syncFilesForPlatformSafe(data: IPreviewAppLiveSyncData, filesData: IPreviewAppFilesData, platform: string, device: Device): Promise<void> {
223+
const deviceId = device && device.id || "";
224+
169225
try {
170226
const payloads = this.$previewAppFilesService.getFilesPayload(data, filesData, platform);
227+
payloads.deviceId = deviceId;
171228
if (payloads && payloads.files && payloads.files.length) {
172-
this.$logger.info(`Start syncing changes for platform ${platform}.`);
229+
this.$logger.info(`Start syncing changes for device ${this.getDeviceDisplayName(device)}.`);
173230
await this.$previewSdkService.applyChanges(payloads);
174-
this.$logger.info(`Successfully synced ${payloads.files.map(filePayload => filePayload.file.yellow)} for platform ${platform}.`);
231+
this.$logger.info(`Successfully synced '${payloads.files.map(filePayload => filePayload.file.yellow)}' for device ${this.getDeviceDisplayName(device)}.`);
175232
}
176233
} catch (error) {
177-
this.$logger.warn(`Unable to apply changes for platform ${platform}. Error is: ${error}, ${JSON.stringify(error, null, 2)}.`);
234+
this.$logger.warn(`Unable to apply changes for device ${this.getDeviceDisplayName(device)}. Error is: ${error}, ${JSON.stringify(error, null, 2)}.`);
178235
this.emit(PreviewAppLiveSyncEvents.PREVIEW_APP_LIVE_SYNC_ERROR, {
179236
error,
180237
data,

‎lib/controllers/run-controller.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,8 @@ export class RunController extends EventEmitter implements IRunController {
396396

397397
if (!liveSyncResultInfo.didRecover && isInHMRMode) {
398398
const status = await this.$hmrStatusService.getHmrStatus(device.deviceInfo.identifier, data.hmrData.hash);
399-
if (status === HmrConstants.HMR_ERROR_STATUS) {
399+
// error or timeout
400+
if (status !== HmrConstants.HMR_SUCCESS_STATUS) {
400401
watchInfo.filesToSync = data.hmrData.fallbackFiles;
401402
liveSyncResultInfo = await platformLiveSyncService.liveSyncWatchAction(device, watchInfo);
402403
// We want to force a restart of the application.

‎lib/definitions/preview-app-livesync.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ declare global {
1414

1515
interface IPreviewAppLiveSyncData extends IProjectDir, IHasUseHotModuleReloadOption, IEnvOptions { }
1616

17-
interface IPreviewSdkService extends EventEmitter {
17+
interface IPreviewSdkService {
1818
getQrCodeUrl(options: IGetQrCodeUrlOptions): string;
1919
initialize(projectDir: string, getInitialFiles: (device: Device) => Promise<FilesPayload>): void;
2020
applyChanges(filesPayload: FilesPayload): Promise<void>;

‎lib/services/livesync/playground/preview-sdk-service.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { MessagingService, Config, Device, DeviceConnectedMessage, SdkCallbacks, ConnectedDevices, FilesPayload } from "nativescript-preview-sdk";
2-
import { EventEmitter } from "events";
32
const pako = require("pako");
43

5-
export class PreviewSdkService extends EventEmitter implements IPreviewSdkService {
4+
export class PreviewSdkService implements IPreviewSdkService {
65
private static MAX_FILES_UPLOAD_BYTE_LENGTH = 15 * 1024 * 1024; // In MBs
76
private messagingService: MessagingService = null;
87
private instanceId: string = null;
@@ -13,7 +12,6 @@ export class PreviewSdkService extends EventEmitter implements IPreviewSdkServic
1312
private $previewDevicesService: IPreviewDevicesService,
1413
private $previewAppLogProvider: IPreviewAppLogProvider,
1514
private $previewSchemaService: IPreviewSchemaService) {
16-
super();
1715
}
1816

1917
public getQrCodeUrl(options: IGetQrCodeUrlOptions): string {

‎lib/services/log-parser-service.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export class LogParserService extends EventEmitter implements ILogParserService
77

88
constructor(private $deviceLogProvider: Mobile.IDeviceLogProvider,
99
private $errors: IErrors,
10-
private $previewSdkService: IPreviewSdkService) {
10+
private $previewAppLogProvider: IPreviewAppLogProvider) {
1111
super();
1212
}
1313

@@ -23,10 +23,12 @@ export class LogParserService extends EventEmitter implements ILogParserService
2323
@cache()
2424
private startParsingLogCore(): void {
2525
this.$deviceLogProvider.on(DEVICE_LOG_EVENT_NAME, this.processDeviceLogResponse.bind(this));
26-
this.$previewSdkService.on(DEVICE_LOG_EVENT_NAME, this.processDeviceLogResponse.bind(this));
26+
this.$previewAppLogProvider.on(DEVICE_LOG_EVENT_NAME, (deviceId: string, message: string) => {
27+
this.processDeviceLogResponse(message, deviceId);
28+
});
2729
}
2830

29-
private processDeviceLogResponse(message: string, deviceIdentifier: string, devicePlatform: string) {
31+
private processDeviceLogResponse(message: string, deviceIdentifier: string, devicePlatform?: string) {
3032
const lines = message.split("\n");
3133
_.forEach(lines, line => {
3234
_.forEach(this.parseRules, parseRule => {

‎test/services/ios-debugger-port-service.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const device = <Mobile.IDevice>{
3030
function createTestInjector() {
3131
const injector = new Yok();
3232

33+
injector.register("previewAppLogProvider", { on: () => ({}) });
3334
injector.register("devicePlatformsConstants", DevicePlatformsConstants);
3435
injector.register("deviceLogProvider", DeveiceLogProviderMock);
3536
injector.register("errors", ErrorsStub);

‎test/services/log-parser-service.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class DeveiceLogProviderMock extends EventEmitter {
2020

2121
function createTestInjector() {
2222
const injector = new Yok();
23+
injector.register("previewAppLogProvider", { on: () => ({}) });
2324
injector.register("deviceLogProvider", DeveiceLogProviderMock);
2425
injector.register("previewSdkService", {
2526
on: () => ({})

0 commit comments

Comments
 (0)