From 6bd024a67e9eb4fd303b14f6c79d7277e854068e Mon Sep 17 00:00:00 2001 From: DimitarTachev Date: Fri, 25 Oct 2019 11:40:40 +0300 Subject: [PATCH 1/4] feat: recover after HMR crashes in Preview Additional Details and Improvements: - no connected devices info is logged when there aren't any connected devices - a hint for starting the Preview app is logged on HMR status timeout - the device specific logs are printed with a device id prefix - run with HMR is now assuming the Status timeout as HMR error and retrying with a full sync - HMR status reading is fixed - we are attaching to the proper Preview logs provider as the previewSdkService is not emitting any logs - HMR updates are now chained in order to avoid getting wrong status from parallel updates - HMR updates are batched in order to avoid slower updates on fast changes --- lib/controllers/preview-app-controller.ts | 117 +++++++++++++----- lib/controllers/run-controller.ts | 3 +- lib/definitions/preview-app-livesync.d.ts | 2 +- .../playground/preview-sdk-service.ts | 4 +- lib/services/log-parser-service.ts | 8 +- test/services/ios-debugger-port-service.ts | 1 + test/services/log-parser-service.ts | 1 + 7 files changed, 94 insertions(+), 42 deletions(-) diff --git a/lib/controllers/preview-app-controller.ts b/lib/controllers/preview-app-controller.ts index 7a9929ac02..d6a2c092ec 100644 --- a/lib/controllers/preview-app-controller.ts +++ b/lib/controllers/preview-app-controller.ts @@ -1,8 +1,8 @@ -import { Device, FilesPayload } from "nativescript-preview-sdk"; import { TrackActionNames, PREPARE_READY_EVENT_NAME } from "../constants"; import { PrepareController } from "./prepare-controller"; +import { Device, FilesPayload } from "nativescript-preview-sdk"; import { performanceLog } from "../common/decorators"; -import { stringify } from "../common/helpers"; +import { stringify, deferPromise } from "../common/helpers"; import { HmrConstants } from "../common/constants"; import { EventEmitter } from "events"; import { PrepareDataService } from "../services/prepare-data-service"; @@ -11,7 +11,12 @@ import { PreviewAppLiveSyncEvents } from "../services/livesync/playground/previe export class PreviewAppController extends EventEmitter implements IPreviewAppController { private prepareReadyEventHandler: any = null; private deviceInitializationPromise: IDictionary = {}; - private promise = Promise.resolve(); + private devicesLiveSyncChain: IDictionary> = {}; + private devicesCanExecuteHmr: IDictionary = {}; + // holds HMR files per device in order to execute batch upload on fast updates + private devicesHmrFiles: IDictionary = {}; + // holds the current HMR hash per device in order to watch the proper hash status on fast updates + private devicesCurrentHmrHash: IDictionary = {}; constructor( private $analyticsService: IAnalyticsService, @@ -89,6 +94,7 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon if (data.useHotModuleReload) { this.$hmrStatusService.attachToHmrStatusEvent(); + this.devicesCanExecuteHmr[device.id] = true; } await this.$previewAppPluginsService.comparePluginsOnDevice(data, device); @@ -109,13 +115,13 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon await this.$prepareController.prepare(prepareData); try { - const payloads = await this.getInitialFilesForPlatformSafe(data, device.platform); + const payloads = await this.getInitialFilesForDeviceSafe(data, device); return payloads; } finally { this.deviceInitializationPromise[device.id] = null; } } catch (error) { - this.$logger.trace(`Error while sending files on device ${device && device.id}. Error is`, error); + this.$logger.trace(`Error while sending files on device '${device && device.id}'. Error is`, error); this.emit(PreviewAppLiveSyncEvents.PREVIEW_APP_LIVE_SYNC_ERROR, { error, data, @@ -129,52 +135,95 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon @performanceLog() private async handlePrepareReadyEvent(data: IPreviewAppLiveSyncData, currentPrepareData: IFilesChangeEventData) { - await this.promise - .then(async () => { - const { hmrData, files, platform } = currentPrepareData; - const platformHmrData = _.cloneDeep(hmrData); - - this.promise = this.syncFilesForPlatformSafe(data, { filesToSync: files }, platform); - await this.promise; - - if (data.useHotModuleReload && platformHmrData.hash) { - const devices = this.$previewDevicesService.getDevicesForPlatform(platform); - - await Promise.all(_.map(devices, async (previewDevice: Device) => { - const status = await this.$hmrStatusService.getHmrStatus(previewDevice.id, platformHmrData.hash); - if (status === HmrConstants.HMR_ERROR_STATUS) { - const originalUseHotModuleReload = data.useHotModuleReload; - data.useHotModuleReload = false; - await this.syncFilesForPlatformSafe(data, { filesToSync: platformHmrData.fallbackFiles }, platform, previewDevice.id); - data.useHotModuleReload = originalUseHotModuleReload; - } - })); + const { hmrData, files, platform } = currentPrepareData; + const platformHmrData = _.cloneDeep(hmrData); + const connectedDevices = this.$previewDevicesService.getDevicesForPlatform(platform); + if (!connectedDevices || !connectedDevices.length) { + this.$logger.warn("Unable to find any connected devices. In order to execute live sync, open your Preview app and optionally re-scan the QR code using the Playground app."); + return; + } + + await Promise.all(_.map(connectedDevices, async (device) => { + const previousSync = this.devicesLiveSyncChain[device.id] || Promise.resolve(); + const currentSyncDeferPromise = deferPromise(); + this.devicesLiveSyncChain[device.id] = currentSyncDeferPromise.promise; + this.devicesCurrentHmrHash[device.id] = this.devicesCurrentHmrHash[device.id] || platformHmrData.hash; + this.devicesHmrFiles[device.id] = this.devicesHmrFiles[device.id] || []; + this.devicesHmrFiles[device.id].push(...files); + await previousSync; + if (!this.devicesHmrFiles[device.id] || !this.devicesHmrFiles[device.id].length) { + this.$logger.info("Skipping files sync. The changes are already batch transferred in a previous sync."); + currentSyncDeferPromise.resolve(); + return; + } + + try { + + let executeHmrSync = false; + const hmrHash = this.devicesCurrentHmrHash[device.id]; + this.devicesCurrentHmrHash[device.id] = null; + if (data.useHotModuleReload && hmrHash) { + if (this.devicesCanExecuteHmr[device.id]) { + executeHmrSync = true; + this.$hmrStatusService.watchHmrStatus(device.id, hmrHash); + } } - }); + + const filesToSync = executeHmrSync ? this.devicesHmrFiles[device.id] : platformHmrData.fallbackFiles; + this.devicesHmrFiles[device.id] = []; + if (executeHmrSync) { + await this.syncFilesForPlatformSafe(data, { filesToSync }, platform, device); + const status = await this.$hmrStatusService.getHmrStatus(device.id, hmrHash); + if (!status) { + this.devicesCanExecuteHmr[device.id] = false; + const noStatusWarning = this.getDeviceMsg(device.name, + "Unable to get LiveSync status from the Preview app. Ensure the app is running in order to sync changes."); + this.$logger.warn(noStatusWarning); + } else { + this.devicesCanExecuteHmr[device.id] = status === HmrConstants.HMR_SUCCESS_STATUS; + } + } else { + const noHmrData = _.assign({}, data, { useHotModuleReload: false }); + await this.syncFilesForPlatformSafe(noHmrData, { filesToSync }, platform, device); + this.devicesCanExecuteHmr[device.id] = true; + } + currentSyncDeferPromise.resolve(); + } catch (e) { + currentSyncDeferPromise.resolve(); + } + })); } - private async getInitialFilesForPlatformSafe(data: IPreviewAppLiveSyncData, platform: string): Promise { - this.$logger.info(`Start sending initial files for platform ${platform}.`); + private getDeviceMsg(deviceId: string, message: string) { + return `[${deviceId}] ${message}`; + } + + private async getInitialFilesForDeviceSafe(data: IPreviewAppLiveSyncData, device: Device): Promise { + const platform = device.platform; + this.$logger.info(`Start sending initial files for device '${device.name}'.`); try { const payloads = this.$previewAppFilesService.getInitialFilesPayload(data, platform); - this.$logger.info(`Successfully sent initial files for platform ${platform}.`); + this.$logger.info(`Successfully sent initial files for device '${device.name}'.`); return payloads; } catch (err) { - this.$logger.warn(`Unable to apply changes for platform ${platform}. Error is: ${err}, ${stringify(err)}`); + this.$logger.warn(`Unable to apply changes for device '${device.name}'. Error is: ${err}, ${stringify(err)}`); } } - private async syncFilesForPlatformSafe(data: IPreviewAppLiveSyncData, filesData: IPreviewAppFilesData, platform: string, deviceId?: string): Promise { + private async syncFilesForPlatformSafe(data: IPreviewAppLiveSyncData, filesData: IPreviewAppFilesData, platform: string, device: Device): Promise { + const deviceId = device && device.id || ""; + try { const payloads = this.$previewAppFilesService.getFilesPayload(data, filesData, platform); + payloads.deviceId = deviceId; if (payloads && payloads.files && payloads.files.length) { - this.$logger.info(`Start syncing changes for platform ${platform}.`); + this.$logger.info(`Start syncing changes for device '${device.name}'.`); await this.$previewSdkService.applyChanges(payloads); - this.$logger.info(`Successfully synced ${payloads.files.map(filePayload => filePayload.file.yellow)} for platform ${platform}.`); + this.$logger.info(`Successfully synced '${payloads.files.map(filePayload => filePayload.file.yellow)}' for device '${device.name}'.`); } } catch (error) { - this.$logger.warn(`Unable to apply changes for platform ${platform}. Error is: ${error}, ${JSON.stringify(error, null, 2)}.`); + this.$logger.warn(`Unable to apply changes for device '${device.name}'. Error is: ${error}, ${JSON.stringify(error, null, 2)}.`); this.emit(PreviewAppLiveSyncEvents.PREVIEW_APP_LIVE_SYNC_ERROR, { error, data, diff --git a/lib/controllers/run-controller.ts b/lib/controllers/run-controller.ts index caee785a21..fd2a8f2c60 100644 --- a/lib/controllers/run-controller.ts +++ b/lib/controllers/run-controller.ts @@ -396,7 +396,8 @@ export class RunController extends EventEmitter implements IRunController { if (!liveSyncResultInfo.didRecover && isInHMRMode) { const status = await this.$hmrStatusService.getHmrStatus(device.deviceInfo.identifier, data.hmrData.hash); - if (status === HmrConstants.HMR_ERROR_STATUS) { + // error or timeout + if (status !== HmrConstants.HMR_SUCCESS_STATUS) { watchInfo.filesToSync = data.hmrData.fallbackFiles; liveSyncResultInfo = await platformLiveSyncService.liveSyncWatchAction(device, watchInfo); // We want to force a restart of the application. diff --git a/lib/definitions/preview-app-livesync.d.ts b/lib/definitions/preview-app-livesync.d.ts index ef51843024..467da95e92 100644 --- a/lib/definitions/preview-app-livesync.d.ts +++ b/lib/definitions/preview-app-livesync.d.ts @@ -14,7 +14,7 @@ declare global { interface IPreviewAppLiveSyncData extends IProjectDir, IHasUseHotModuleReloadOption, IEnvOptions { } - interface IPreviewSdkService extends EventEmitter { + interface IPreviewSdkService { getQrCodeUrl(options: IGetQrCodeUrlOptions): string; initialize(projectDir: string, getInitialFiles: (device: Device) => Promise): void; applyChanges(filesPayload: FilesPayload): Promise; diff --git a/lib/services/livesync/playground/preview-sdk-service.ts b/lib/services/livesync/playground/preview-sdk-service.ts index 63e805b067..48cb48b2ce 100644 --- a/lib/services/livesync/playground/preview-sdk-service.ts +++ b/lib/services/livesync/playground/preview-sdk-service.ts @@ -1,8 +1,7 @@ import { MessagingService, Config, Device, DeviceConnectedMessage, SdkCallbacks, ConnectedDevices, FilesPayload } from "nativescript-preview-sdk"; -import { EventEmitter } from "events"; const pako = require("pako"); -export class PreviewSdkService extends EventEmitter implements IPreviewSdkService { +export class PreviewSdkService implements IPreviewSdkService { private static MAX_FILES_UPLOAD_BYTE_LENGTH = 15 * 1024 * 1024; // In MBs private messagingService: MessagingService = null; private instanceId: string = null; @@ -13,7 +12,6 @@ export class PreviewSdkService extends EventEmitter implements IPreviewSdkServic private $previewDevicesService: IPreviewDevicesService, private $previewAppLogProvider: IPreviewAppLogProvider, private $previewSchemaService: IPreviewSchemaService) { - super(); } public getQrCodeUrl(options: IGetQrCodeUrlOptions): string { diff --git a/lib/services/log-parser-service.ts b/lib/services/log-parser-service.ts index 97cf19c410..5d88bf2517 100644 --- a/lib/services/log-parser-service.ts +++ b/lib/services/log-parser-service.ts @@ -7,7 +7,7 @@ export class LogParserService extends EventEmitter implements ILogParserService constructor(private $deviceLogProvider: Mobile.IDeviceLogProvider, private $errors: IErrors, - private $previewSdkService: IPreviewSdkService) { + private $previewAppLogProvider: IPreviewAppLogProvider) { super(); } @@ -23,10 +23,12 @@ export class LogParserService extends EventEmitter implements ILogParserService @cache() private startParsingLogCore(): void { this.$deviceLogProvider.on(DEVICE_LOG_EVENT_NAME, this.processDeviceLogResponse.bind(this)); - this.$previewSdkService.on(DEVICE_LOG_EVENT_NAME, this.processDeviceLogResponse.bind(this)); + this.$previewAppLogProvider.on(DEVICE_LOG_EVENT_NAME, (deviceId: string, message: string) => { + this.processDeviceLogResponse(message, deviceId); + }); } - private processDeviceLogResponse(message: string, deviceIdentifier: string, devicePlatform: string) { + private processDeviceLogResponse(message: string, deviceIdentifier: string, devicePlatform?: string) { const lines = message.split("\n"); _.forEach(lines, line => { _.forEach(this.parseRules, parseRule => { diff --git a/test/services/ios-debugger-port-service.ts b/test/services/ios-debugger-port-service.ts index 512ea74ebf..e3edad7996 100644 --- a/test/services/ios-debugger-port-service.ts +++ b/test/services/ios-debugger-port-service.ts @@ -30,6 +30,7 @@ const device = { function createTestInjector() { const injector = new Yok(); + injector.register("previewAppLogProvider", { on: () => ({}) }); injector.register("devicePlatformsConstants", DevicePlatformsConstants); injector.register("deviceLogProvider", DeveiceLogProviderMock); injector.register("errors", ErrorsStub); diff --git a/test/services/log-parser-service.ts b/test/services/log-parser-service.ts index ec606349db..bd6bde2b5f 100644 --- a/test/services/log-parser-service.ts +++ b/test/services/log-parser-service.ts @@ -20,6 +20,7 @@ class DeveiceLogProviderMock extends EventEmitter { function createTestInjector() { const injector = new Yok(); + injector.register("previewAppLogProvider", { on: () => ({}) }); injector.register("deviceLogProvider", DeveiceLogProviderMock); injector.register("previewSdkService", { on: () => ({}) From 1437793d362e3f1e2e260705e1f343b73086f138 Mon Sep 17 00:00:00 2001 From: DimitarTachev Date: Thu, 31 Oct 2019 15:49:47 +0200 Subject: [PATCH 2/4] refactor: improve the devices display name in preview --- lib/controllers/preview-app-controller.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/controllers/preview-app-controller.ts b/lib/controllers/preview-app-controller.ts index d6a2c092ec..9975d0a9a9 100644 --- a/lib/controllers/preview-app-controller.ts +++ b/lib/controllers/preview-app-controller.ts @@ -176,9 +176,7 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon const status = await this.$hmrStatusService.getHmrStatus(device.id, hmrHash); if (!status) { this.devicesCanExecuteHmr[device.id] = false; - const noStatusWarning = this.getDeviceMsg(device.name, - "Unable to get LiveSync status from the Preview app. Ensure the app is running in order to sync changes."); - this.$logger.warn(noStatusWarning); + 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.`); } else { this.devicesCanExecuteHmr[device.id] = status === HmrConstants.HMR_SUCCESS_STATUS; } @@ -194,20 +192,20 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon })); } - private getDeviceMsg(deviceId: string, message: string) { - return `[${deviceId}] ${message}`; + private getDeviceDisplayName(device: Device) { + return `${device.name} (${device.id})`.cyan; } private async getInitialFilesForDeviceSafe(data: IPreviewAppLiveSyncData, device: Device): Promise { const platform = device.platform; - this.$logger.info(`Start sending initial files for device '${device.name}'.`); + this.$logger.info(`Start sending initial files for device ${this.getDeviceDisplayName(device)}.`); try { const payloads = this.$previewAppFilesService.getInitialFilesPayload(data, platform); - this.$logger.info(`Successfully sent initial files for device '${device.name}'.`); + this.$logger.info(`Successfully sent initial files for device ${this.getDeviceDisplayName(device)}.`); return payloads; } catch (err) { - this.$logger.warn(`Unable to apply changes for device '${device.name}'. Error is: ${err}, ${stringify(err)}`); + this.$logger.warn(`Unable to apply changes for device ${this.getDeviceDisplayName(device)}. Error is: ${err}, ${stringify(err)}`); } } @@ -218,12 +216,12 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon const payloads = this.$previewAppFilesService.getFilesPayload(data, filesData, platform); payloads.deviceId = deviceId; if (payloads && payloads.files && payloads.files.length) { - this.$logger.info(`Start syncing changes for device '${device.name}'.`); + this.$logger.info(`Start syncing changes for device ${this.getDeviceDisplayName(device)}.`); await this.$previewSdkService.applyChanges(payloads); - this.$logger.info(`Successfully synced '${payloads.files.map(filePayload => filePayload.file.yellow)}' for device '${device.name}'.`); + this.$logger.info(`Successfully synced '${payloads.files.map(filePayload => filePayload.file.yellow)}' for device ${this.getDeviceDisplayName(device)}.`); } } catch (error) { - this.$logger.warn(`Unable to apply changes for device '${device.name}'. Error is: ${error}, ${JSON.stringify(error, null, 2)}.`); + this.$logger.warn(`Unable to apply changes for device ${this.getDeviceDisplayName(device)}. Error is: ${error}, ${JSON.stringify(error, null, 2)}.`); this.emit(PreviewAppLiveSyncEvents.PREVIEW_APP_LIVE_SYNC_ERROR, { error, data, From b1dba617c95f773d83736169a61f0fdd956667d8 Mon Sep 17 00:00:00 2001 From: DimitarTachev Date: Fri, 1 Nov 2019 11:09:06 +0200 Subject: [PATCH 3/4] refactor: improve no connected devices warning --- lib/controllers/preview-app-controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/controllers/preview-app-controller.ts b/lib/controllers/preview-app-controller.ts index 9975d0a9a9..5b68e255e5 100644 --- a/lib/controllers/preview-app-controller.ts +++ b/lib/controllers/preview-app-controller.ts @@ -139,7 +139,7 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon const platformHmrData = _.cloneDeep(hmrData); const connectedDevices = this.$previewDevicesService.getDevicesForPlatform(platform); if (!connectedDevices || !connectedDevices.length) { - this.$logger.warn("Unable to find any connected devices. In order to execute live sync, open your Preview app and optionally re-scan the QR code using the Playground app."); + 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.`); return; } From 373593a375b3ac3a37ee4a5cd178ba7134cbf7ae Mon Sep 17 00:00:00 2001 From: DimitarTachev Date: Wed, 20 Nov 2019 11:56:38 +0200 Subject: [PATCH 4/4] fix: sync all files instead of the hmrFallbackFiles in --no-hmr flow --- lib/controllers/preview-app-controller.ts | 36 +++++++++++++++-------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/lib/controllers/preview-app-controller.ts b/lib/controllers/preview-app-controller.ts index 5b68e255e5..e568475a5b 100644 --- a/lib/controllers/preview-app-controller.ts +++ b/lib/controllers/preview-app-controller.ts @@ -15,6 +15,8 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon private devicesCanExecuteHmr: IDictionary = {}; // holds HMR files per device in order to execute batch upload on fast updates private devicesHmrFiles: IDictionary = {}; + // holds app files per device in order to execute batch upload on fast updates on failed HMR or --no-hmr + private devicesAppFiles: IDictionary = {}; // holds the current HMR hash per device in order to watch the proper hash status on fast updates private devicesCurrentHmrHash: IDictionary = {}; @@ -148,30 +150,38 @@ export class PreviewAppController extends EventEmitter implements IPreviewAppCon const currentSyncDeferPromise = deferPromise(); this.devicesLiveSyncChain[device.id] = currentSyncDeferPromise.promise; this.devicesCurrentHmrHash[device.id] = this.devicesCurrentHmrHash[device.id] || platformHmrData.hash; - this.devicesHmrFiles[device.id] = this.devicesHmrFiles[device.id] || []; - this.devicesHmrFiles[device.id].push(...files); - await previousSync; - if (!this.devicesHmrFiles[device.id] || !this.devicesHmrFiles[device.id].length) { - this.$logger.info("Skipping files sync. The changes are already batch transferred in a previous sync."); - currentSyncDeferPromise.resolve(); - return; + if (data.useHotModuleReload) { + this.devicesHmrFiles[device.id] = this.devicesHmrFiles[device.id] || []; + this.devicesHmrFiles[device.id].push(...files); + this.devicesAppFiles[device.id] = platformHmrData.fallbackFiles; + } else { + this.devicesHmrFiles[device.id] = []; + this.devicesAppFiles[device.id] = files; } - try { + await previousSync; - let executeHmrSync = false; + try { + let canExecuteHmrSync = false; const hmrHash = this.devicesCurrentHmrHash[device.id]; this.devicesCurrentHmrHash[device.id] = null; if (data.useHotModuleReload && hmrHash) { if (this.devicesCanExecuteHmr[device.id]) { - executeHmrSync = true; - this.$hmrStatusService.watchHmrStatus(device.id, hmrHash); + canExecuteHmrSync = true; } } - const filesToSync = executeHmrSync ? this.devicesHmrFiles[device.id] : platformHmrData.fallbackFiles; + const filesToSync = canExecuteHmrSync ? this.devicesHmrFiles[device.id] : this.devicesAppFiles[device.id]; + if (!filesToSync || !filesToSync.length) { + this.$logger.info(`Skipping files sync for device ${this.getDeviceDisplayName(device)}. The changes are already batch transferred in a previous sync.`); + currentSyncDeferPromise.resolve(); + return; + } + this.devicesHmrFiles[device.id] = []; - if (executeHmrSync) { + this.devicesAppFiles[device.id] = []; + if (canExecuteHmrSync) { + this.$hmrStatusService.watchHmrStatus(device.id, hmrHash); await this.syncFilesForPlatformSafe(data, { filesToSync }, platform, device); const status = await this.$hmrStatusService.getHmrStatus(device.id, hmrHash); if (!status) {