From d5fbe9a5b9318aa623ed7fbd5d7bd30147d9bb03 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 29 Mar 2021 06:59:15 -0700 Subject: [PATCH] Support physical device logging Summary: Changelog: Logs plugin now supports physical iOS devices As reported in https://github.com/facebook/flipper/issues/262 and linked papercut. This diff adds support for iOS device logs through idb. Since idb doesn't respect `--json` flag at the moment, we perform the parsing in Flipper itself. Reviewed By: passy Differential Revision: D27346262 fbshipit-source-id: 3b314716f48bb9a7fe709370303396a51893359c --- desktop/app/package.json | 2 + desktop/app/src/devices/IOSDevice.tsx | 82 ++++++++++++++---- .../src/devices/__tests__/iOSLogs.node.tsx | Bin 0 -> 2832 bytes .../dispatcher/__tests__/iOSDevice.node.tsx | 8 +- desktop/app/src/utils/IOSBridge.tsx | 51 +++++++---- .../src/utils/__tests__/IOSBridge.node.tsx | 29 ++++++- desktop/plugins/logs/index.tsx | 4 +- desktop/plugins/logs/package.json | 23 ++++- desktop/yarn.lock | 16 +++- 9 files changed, 170 insertions(+), 45 deletions(-) create mode 100644 desktop/app/src/devices/__tests__/iOSLogs.node.tsx diff --git a/desktop/app/package.json b/desktop/app/package.json index 315358858..847cac610 100644 --- a/desktop/app/package.json +++ b/desktop/app/package.json @@ -72,6 +72,7 @@ "rsocket-tcp-server": "^0.0.19", "rsocket-types": "^0.0.16", "semver": "^7.3.5", + "split2": "^3.2.2", "string-natural-compare": "^3.0.0", "tmp": "^0.2.1", "uuid": "^8.3.2", @@ -86,6 +87,7 @@ "@testing-library/dom": "^7.30.1", "@testing-library/react": "^11.2.5", "@types/lodash.memoize": "^4.1.6", + "@types/split2": "^2.1.6", "flipper-test-utils": "0.0.0", "metro-runtime": "^0.65.2", "mock-fs": "^4.13.0", diff --git a/desktop/app/src/devices/IOSDevice.tsx b/desktop/app/src/devices/IOSDevice.tsx index 44324d8d3..1923d8762 100644 --- a/desktop/app/src/devices/IOSDevice.tsx +++ b/desktop/app/src/devices/IOSDevice.tsx @@ -20,6 +20,7 @@ import {promisify} from 'util'; import {exec} from 'child_process'; import {default as promiseTimeout} from '../utils/promiseTimeout'; import {IOSBridge} from '../utils/IOSBridge'; +import split2 from 'split2'; type IOSLogLevel = 'Default' | 'Info' | 'Debug' | 'Error' | 'Fault'; @@ -40,6 +41,10 @@ type RawLogEntry = { traceID: string; }; +// https://regex101.com/r/rrl03T/1 +// Mar 25 17:06:38 iPhone symptomsd(SymptomEvaluator)[125] : Stuff +const logRegex = /(^.{15}) ([^ ]+?) ([^\[]+?)\[(\d+?)\] <(\w+?)>: (.*)$/s; + export default class IOSDevice extends BaseDevice { log?: child_process.ChildProcessWithoutNullStreams; buffer: string; @@ -97,8 +102,13 @@ export default class IOSDevice extends BaseDevice { } const logListener = iOSBridge.startLogListener; - if (!this.log && logListener) { - this.log = logListener(this.serial); + if ( + !this.log && + logListener && + (this.deviceType === 'emulator' || + (this.deviceType === 'physical' && iOSBridge.idbAvailable)) + ) { + this.log = logListener(this.serial, this.deviceType); this.log.on('error', (err: Error) => { console.error('iOS log tailer error', err); }); @@ -112,13 +122,24 @@ export default class IOSDevice extends BaseDevice { }); try { - this.log.stdout - .pipe(new StripLogPrefix()) - .pipe(JSONStream.parse('*')) - .on('data', (data: RawLogEntry) => { - const entry = IOSDevice.parseLogEntry(data); - this.addLogEntry(entry); + if (this.deviceType === 'physical') { + this.log.stdout.pipe(split2('\0')).on('data', (line: string) => { + const parsed = IOSDevice.parseLogLine(line); + if (parsed) { + this.addLogEntry(parsed); + } else { + console.warn('Failed to parse iOS log line: ', line); + } }); + } else { + this.log.stdout + .pipe(new StripLogPrefix()) + .pipe(JSONStream.parse('*')) + .on('data', (data: RawLogEntry) => { + const entry = IOSDevice.parseJsonLogEntry(data); + this.addLogEntry(entry); + }); + } } catch (e) { console.error('Could not parse iOS log stream.', e); // restart log stream @@ -129,15 +150,42 @@ export default class IOSDevice extends BaseDevice { } } - static parseLogEntry(entry: RawLogEntry): DeviceLogEntry { - const LOG_MAPPING: Map = new Map([ - ['Default' as IOSLogLevel, 'debug' as LogLevel], - ['Info' as IOSLogLevel, 'info' as LogLevel], - ['Debug' as IOSLogLevel, 'debug' as LogLevel], - ['Error' as IOSLogLevel, 'error' as LogLevel], - ['Fault' as IOSLogLevel, 'fatal' as LogLevel], - ]); - let type: LogLevel = LOG_MAPPING.get(entry.messageType) || 'unknown'; + static getLogLevel(level: string): LogLevel { + switch (level) { + case 'Default': + return 'debug'; + case 'Info': + return 'info'; + case 'Debug': + return 'debug'; + case 'Error': + return 'error'; + case 'Notice': + return 'verbose'; + case 'Fault': + return 'fatal'; + default: + return 'unknown'; + } + } + + static parseLogLine(line: string): DeviceLogEntry | undefined { + const matches = line.match(logRegex); + if (matches) { + return { + date: new Date(Date.parse(matches[1])), + tag: matches[3], + tid: 0, + pid: parseInt(matches[4], 10), + type: IOSDevice.getLogLevel(matches[5]), + message: matches[6], + }; + } + return undefined; + } + + static parseJsonLogEntry(entry: RawLogEntry): DeviceLogEntry { + let type: LogLevel = IOSDevice.getLogLevel(entry.messageType); // when Apple log levels are not used, log messages can be prefixed with // their loglevel. diff --git a/desktop/app/src/devices/__tests__/iOSLogs.node.tsx b/desktop/app/src/devices/__tests__/iOSLogs.node.tsx new file mode 100644 index 0000000000000000000000000000000000000000..7348ea0fe9dcdad0243fafe0e67ec40335292d0e GIT binary patch literal 2832 zcmdT`ZI9wM5bo#w74wbc1jGp;K#tJk^|TUj+he8sKv$ee47eNO$ToX$Rrj}d9A1{| z%c`nW=_&-o;~9HA^E{8^ym@m2Z=f&ogXHUt1}nB z)v9wbmEPXmVE)=F#WaJQO2r@*>rxpk8=?;UdINZnOleFd-)V^8^Mp$9>fkm)5`?uT z@Vkx37^uTG*J7&@Yj%7ZKGAeXF=2b|di4irPld)LIw6cbm@qAQtUzSIG7^GRiic1M ze3+9md4NjC49cJ~83Ms3#1!UV=p%Eq&@_Dj#nP1{ux8TXnO@o9G0-W(Prq&emfnvf z-7@9q{YQ4c$G;KfDY_E}KV~263snwRwy}JEdpU1Wiv$mRsa3Twjm3J*GiDX_u#G?4 z=hztQ7+_bmm{1*WJ}iGRR#QL#OhC~!3yT3j>HDofsa)ik^CSdp+i|4nI)u_I9X>^A z6Gcujhn!*16O&7!SmE$nQ7*@UL}Vom@rK2Zz|iA>aL(31Re?bBi0IaQ%iyblR>E?I z+{qBu-xmAFYJ%*mu9=>QOm8gPRKLe8@6Ua?a^=|$A%AfWUUQx?M1S()Tx+7YImd}L zG)<4Jq01Bx`PZybh@yE>)+2cWa7jo==#w*f9+}KoYIXBGWxf97)3`hB3(4#`QsIg> zKTRs1_y~z&dGBqI`@U4~Q;uMZeA2>kFW($qY@QdOZKkQzQ9WpQ#(^ex<*p{w#7En3vp-_)3ruPpdRKkKD^>k%{GQRZMiV!%)h|Z(_(trgGE@c8{GCI~@hC z*=8yREaltCH7+B$y%%CEvh{JN5k^HdMI2S2XJDDnE633RzUDCX+ z&2%t3&Oo~{bemm|G%Gzas#SbK29=f@3@hD1)Aze0w?#%?2kxnQgm%4Fb4R^~S7`*T zVa4xxUZpu4j4HK(-}3#Q+iH+*r`oP}P<|6Y0cQLO0ZBClEAU!PujaJ^Bgk&6Uh`W4 zX#`Lx6f6%2GW&d_ZY}{lAT`r>?8`|sQxQ{L9LI0C-<;=NKi$92RT)NnUq((rnwDYe zbc+QajO-Kwq^cC00QM1MVw;s~iEQ{tnrf`=Efd z7lidow~#lQ;<9)hWhi|znftA#d)+dnsfKwwm;4D?+<6F*++1me!c#^87@Y4rBPp;_ zQ@l?@Xg@zcckuK-6xYzd+179>7F$d!iE!aK<)|!CyI8mkyQp0!u4BH>&yMG~gpdxJ zc@ouXWk6kw>TYo}Vf<3;SrTEkc}lgpc_R;fnUq3CR)*4Lzmh4(TNULau~NQN2c_9| z@yD7~lLJoLbF<@h@j(-qn{$-wWo?R6;t literal 0 HcmV?d00001 diff --git a/desktop/app/src/dispatcher/__tests__/iOSDevice.node.tsx b/desktop/app/src/dispatcher/__tests__/iOSDevice.node.tsx index 2e825213d..98679be75 100644 --- a/desktop/app/src/dispatcher/__tests__/iOSDevice.node.tsx +++ b/desktop/app/src/dispatcher/__tests__/iOSDevice.node.tsx @@ -64,7 +64,9 @@ test('test getAllPromisesForQueryingDevices when xcode detected', () => { const promises = getAllPromisesForQueryingDevices( mockStore, logger, - {}, + { + idbAvailable: false, + }, true, ); expect(promises.length).toEqual(3); @@ -74,7 +76,9 @@ test('test getAllPromisesForQueryingDevices when xcode is not detected', () => { const promises = getAllPromisesForQueryingDevices( mockStore, logger, - {}, + { + idbAvailable: true, + }, false, ); expect(promises.length).toEqual(1); diff --git a/desktop/app/src/utils/IOSBridge.tsx b/desktop/app/src/utils/IOSBridge.tsx index 8c760a3d6..6d16fd94b 100644 --- a/desktop/app/src/utils/IOSBridge.tsx +++ b/desktop/app/src/utils/IOSBridge.tsx @@ -9,10 +9,13 @@ import fs from 'fs'; import child_process from 'child_process'; +import {DeviceType} from 'flipper-plugin-lib'; export interface IOSBridge { + idbAvailable: boolean; startLogListener?: ( udid: string, + deviceType: DeviceType, ) => child_process.ChildProcessWithoutNullStreams; } @@ -26,27 +29,36 @@ async function isAvailable(idbPath: string): Promise { .catch((_) => false); } -const LOG_EXTRA_ARGS = [ - '--style', - 'json', - '--predicate', - 'senderImagePath contains "Containers"', - '--debug', - '--info', -]; +function getLogExtraArgs(deviceType: DeviceType) { + if (deviceType === 'physical') { + return [ + // idb has a --json option, but that doesn't actually work! + ]; + } else { + return [ + '--style', + 'json', + '--predicate', + 'senderImagePath contains "Containers"', + '--debug', + '--info', + ]; + } +} export function idbStartLogListener( idbPath: string, udid: string, + deviceType: DeviceType, ): child_process.ChildProcessWithoutNullStreams { return child_process.spawn( idbPath, - ['log', '--udid', udid, '--', ...LOG_EXTRA_ARGS], + ['log', '--udid', udid, '--', ...getLogExtraArgs(deviceType)], {}, ); } -export function xcrunStartLogListener(udid: string) { +export function xcrunStartLogListener(udid: string, deviceType: DeviceType) { const deviceSetPath = process.env.DEVICE_SET_PATH ? ['--set', process.env.DEVICE_SET_PATH] : []; @@ -59,7 +71,7 @@ export function xcrunStartLogListener(udid: string) { udid, 'log', 'stream', - ...LOG_EXTRA_ARGS, + ...getLogExtraArgs(deviceType), ], {}, ); @@ -70,18 +82,23 @@ export async function makeIOSBridge( isXcodeDetected: boolean, isAvailableFn: (idbPath: string) => Promise = isAvailable, ): Promise { - if (!isXcodeDetected) { - // iOS Physical Device can still get detected without Xcode. In this case there is no way to setup log listener yet. - // This will not be the case, idb team is working on making idb log work without XCode atleast for physical device. - return {}; - } + // prefer idb if (await isAvailableFn(idbPath)) { return { + idbAvailable: true, startLogListener: idbStartLogListener.bind(null, idbPath), }; } + // no idb, if it's a simulator and xcode is available, we can use xcrun + if (isXcodeDetected) { + return { + idbAvailable: false, + startLogListener: xcrunStartLogListener, + }; + } + // no idb, and not a simulator, we can't log this device return { - startLogListener: xcrunStartLogListener, + idbAvailable: false, }; } diff --git a/desktop/app/src/utils/__tests__/IOSBridge.node.tsx b/desktop/app/src/utils/__tests__/IOSBridge.node.tsx index 072abca52..12c2957ee 100644 --- a/desktop/app/src/utils/__tests__/IOSBridge.node.tsx +++ b/desktop/app/src/utils/__tests__/IOSBridge.node.tsx @@ -18,7 +18,7 @@ test('uses xcrun with no idb when xcode is detected', async () => { const ib = await makeIOSBridge('', true); expect(ib.startLogListener).toBeDefined(); - ib.startLogListener!('deadbeef'); + ib.startLogListener!('deadbeef', 'emulator'); expect(spawn).toHaveBeenCalledWith( 'xcrun', @@ -43,7 +43,7 @@ test('uses idb when present and xcode detected', async () => { const ib = await makeIOSBridge('/usr/local/bin/idb', true, async (_) => true); expect(ib.startLogListener).toBeDefined(); - ib.startLogListener!('deadbeef'); + ib.startLogListener!('deadbeef', 'emulator'); expect(spawn).toHaveBeenCalledWith( '/usr/local/bin/idb', @@ -63,6 +63,31 @@ test('uses idb when present and xcode detected', async () => { ); }); +test('uses idb when present and xcode detected and physical device connected', async () => { + const ib = await makeIOSBridge('/usr/local/bin/idb', true, async (_) => true); + expect(ib.startLogListener).toBeDefined(); + + ib.startLogListener!('deadbeef', 'physical'); + + expect(spawn).toHaveBeenCalledWith( + '/usr/local/bin/idb', + [ + 'log', + '--udid', + 'deadbeef', + '--', + // no further args; not supported by idb atm + ], + {}, + ); +}); + +test("without idb physical devices can't log", async () => { + const ib = await makeIOSBridge('', true); + expect(ib.idbAvailable).toBeFalsy(); + expect(ib.startLogListener).toBeDefined(); // since we have xcode +}); + test('uses no log listener when xcode is not detected', async () => { const ib = await makeIOSBridge('', false); expect(ib.startLogListener).toBeUndefined(); diff --git a/desktop/plugins/logs/index.tsx b/desktop/plugins/logs/index.tsx index cef7c6d61..b971d38db 100644 --- a/desktop/plugins/logs/index.tsx +++ b/desktop/plugins/logs/index.tsx @@ -35,7 +35,7 @@ export type ExtendedLogEntry = DeviceLogEntry & { }; function createColumnConfig( - os: 'iOS' | 'Android' | 'Metro', + _os: 'iOS' | 'Android' | 'Metro', ): DataTableColumn[] { return [ { @@ -74,7 +74,7 @@ function createColumnConfig( key: 'pid', title: 'PID', width: 60, - visible: os === 'Android', + visible: true, }, { key: 'tid', diff --git a/desktop/plugins/logs/package.json b/desktop/plugins/logs/package.json index 1a233f421..ab86ab3c7 100644 --- a/desktop/plugins/logs/package.json +++ b/desktop/plugins/logs/package.json @@ -4,10 +4,25 @@ "id": "DeviceLogs", "pluginType": "device", "supportedDevices": [ - {"os": "Android", "type": "emulator"}, - {"os": "Android", "type": "physical"}, - {"os": "iOS", "type": "emulator"}, - {"os": "Metro"} + { + "os": "Android", + "type": "emulator" + }, + { + "os": "Android", + "type": "physical" + }, + { + "os": "iOS", + "type": "emulator" + }, + { + "os": "iOS", + "type": "physical" + }, + { + "os": "Metro" + } ], "version": "0.0.0", "main": "dist/bundle.js", diff --git a/desktop/yarn.lock b/desktop/yarn.lock index 4fa740e22..dea9f7117 100644 --- a/desktop/yarn.lock +++ b/desktop/yarn.lock @@ -2981,6 +2981,13 @@ "@types/node" "*" "@types/socket.io-parser" "*" +"@types/split2@^2.1.6": + version "2.1.6" + resolved "https://registry.yarnpkg.com/@types/split2/-/split2-2.1.6.tgz#b095c9e064853824b22c67993d99b066777402b1" + integrity sha512-ddaFSOMuy2Rp97l6q/LEteQygvTQJuEZ+SRhxFKR0uXGsdbFDqX/QF2xoGcOqLQ8XV91v01SnAv2vpgihNgW/Q== + dependencies: + "@types/node" "*" + "@types/sql-formatter@^2.3.0": version "2.3.0" resolved "https://registry.yarnpkg.com/@types/sql-formatter/-/sql-formatter-2.3.0.tgz#d2584c54f865fd57a7fe7e88ee8ed3623b23da33" @@ -11502,7 +11509,7 @@ readable-stream@^2.0.0, readable-stream@^2.0.5, readable-stream@^2.2.2, readable string_decoder "~1.1.1" util-deprecate "~1.0.1" -readable-stream@^3.1.1, readable-stream@^3.4.0, readable-stream@^3.6.0: +readable-stream@^3.0.0, readable-stream@^3.1.1, readable-stream@^3.4.0, readable-stream@^3.6.0: version "3.6.0" resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-3.6.0.tgz#337bbda3adc0706bd3e024426a286d4b4b2c9198" integrity sha512-BViHy7LKeTz4oNnkcLJ+lVSL6vpiFeX6/d3oSH8zCW7UxP2onchk+vTGB143xuFjHS3deTgkKoXXymXqymiIdA== @@ -12440,6 +12447,13 @@ split-string@^3.0.1, split-string@^3.0.2: dependencies: extend-shallow "^3.0.0" +split2@^3.2.2: + version "3.2.2" + resolved "https://registry.yarnpkg.com/split2/-/split2-3.2.2.tgz#bf2cf2a37d838312c249c89206fd7a17dd12365f" + integrity sha512-9NThjpgZnifTkJpzTZ7Eue85S49QwpNhZTq6GRJwObb6jnLFNGB7Qm73V5HewTROPyxD0C29xqmaI68bQtV+hg== + dependencies: + readable-stream "^3.0.0" + split@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/split/-/split-1.0.1.tgz#605bd9be303aa59fb35f9229fbea0ddec9ea07d9"