From bf9be74ad21776e66861ea1eb47373b9907cd222 Mon Sep 17 00:00:00 2001 From: John Knox Date: Fri, 22 Mar 2019 06:48:29 -0700 Subject: [PATCH] Remove RecurringError type Summary: This was originally introduced so we could squash multiple instances of the same error at the client side and only report one instance of them. Now we've moved to doing the aggregation server side, which is more powerful, so this is no longer necessary. We've also seen a case of these Error objects appearing opaque making it hard to see the underlying problem, so removing it entirely. Reviewed By: passy Differential Revision: D14576715 fbshipit-source-id: b285dcb5249e209f9008a14ac6a2f226f3aa82d7 --- src/dispatcher/iOSDevice.js | 3 +-- src/reducers/connections.js | 11 ++++------- src/server.js | 9 +++------ src/utils/CertificateProvider.js | 27 ++++++++++----------------- src/utils/__tests__/errors.node.js | 16 ---------------- src/utils/errors.js | 17 ----------------- 6 files changed, 18 insertions(+), 65 deletions(-) delete mode 100644 src/utils/__tests__/errors.node.js delete mode 100644 src/utils/errors.js diff --git a/src/dispatcher/iOSDevice.js b/src/dispatcher/iOSDevice.js index 1c01654cb..936c7ab4e 100644 --- a/src/dispatcher/iOSDevice.js +++ b/src/dispatcher/iOSDevice.js @@ -9,7 +9,6 @@ import type {ChildProcess} from 'child_process'; import type {Store} from '../reducers/index.js'; import type {Logger} from '../fb-interfaces/Logger.js'; import type {DeviceType} from '../devices/BaseDevice'; -import {RecurringError} from '../utils/errors'; import {promisify} from 'util'; import path from 'path'; import child_process from 'child_process'; @@ -138,7 +137,7 @@ function getActiveSimulators(): Promise> { function getActiveDevices(): Promise> { return iosUtil.targets().catch(e => { - console.error(new RecurringError(e.message)); + console.error(e.message); return []; }); } diff --git a/src/reducers/connections.js b/src/reducers/connections.js index 8a7f2db1b..0ed7e5be0 100644 --- a/src/reducers/connections.js +++ b/src/reducers/connections.js @@ -9,7 +9,6 @@ import type BaseDevice from '../devices/BaseDevice'; import type Client from '../Client'; import type {UninitializedClient} from '../UninitializedClient'; import {isEqual} from 'lodash'; -import {RecurringError} from '../utils/errors'; import iosUtil from '../fb-stubs/iOSContainerUtility'; // $FlowFixMe perf_hooks is a new API in node import {performance} from 'perf_hooks'; @@ -266,8 +265,7 @@ export default function reducer( ); if (matchingDeviceForClient.length === 0) { console.error( - new RecurringError(`Client initialised for non-displayed device`), - payload.id, + `Client initialised for non-displayed device: ${payload.id}`, ); } } @@ -328,10 +326,9 @@ export default function reducer( const errorMessage = payload.error instanceof Error ? payload.error.message : payload.error; console.error( - new RecurringError(`Client setup error: ${errorMessage}`), - `${payload.client.os}:${payload.client.deviceName}:${ - payload.client.appName - }`, + `Client setup error: ${errorMessage} while setting up client: ${ + payload.client.os + }:${payload.client.deviceName}:${payload.client.appName}`, ); return { ...state, diff --git a/src/server.js b/src/server.js index 31ca010cb..2cebfeb1d 100644 --- a/src/server.js +++ b/src/server.js @@ -16,7 +16,6 @@ import RSocketTCPServer from 'rsocket-tcp-server'; import {Single} from 'rsocket-flowable'; import Client from './Client.js'; import type {UninitializedClient} from './UninitializedClient'; -import {RecurringError} from './utils/errors'; import {reportPlatformFailures} from './utils/metrics'; const EventEmitter = (require('events'): any); @@ -342,11 +341,9 @@ class ConnectionTracker { this.connectionAttempts.set(key, entry); if (entry.length >= this.connectionProblemThreshold) { console.error( - new RecurringError( - `Connection loop detected with ${key}. Connected ${ - this.connectionProblemThreshold - } times within ${this.timeWindowMillis / 1000}s.`, - ), + `Connection loop detected with ${key}. Connected ${ + this.connectionProblemThreshold + } times within ${this.timeWindowMillis / 1000}s.`, 'server', ); } diff --git a/src/utils/CertificateProvider.js b/src/utils/CertificateProvider.js index 5c5167cf9..c96e1b7da 100644 --- a/src/utils/CertificateProvider.js +++ b/src/utils/CertificateProvider.js @@ -6,7 +6,6 @@ */ import type {Logger} from '../fb-interfaces/Logger'; -import {RecurringError} from './errors'; import {promisify} from 'util'; const fs = require('fs'); import { @@ -237,7 +236,7 @@ export default class CertificateProvider { }, ); } - return Promise.reject(new RecurringError(`Unsupported device os: ${os}`)); + return Promise.reject(new Error(`Unsupported device os: ${os}`)); } pushFileToiOSDevice( @@ -284,13 +283,11 @@ export default class CertificateProvider { return Promise.all(deviceMatchList).then(devices => { const matchingIds = devices.filter(m => m.isMatch).map(m => m.id); if (matchingIds.length == 0) { - throw new RecurringError( - `No matching device found for app: ${appName}`, - ); + throw new Error(`No matching device found for app: ${appName}`); } if (matchingIds.length > 1) { console.error( - new RecurringError('More than one matching device found for CSR'), + new Error('More than one matching device found for CSR'), csr, ); } @@ -323,9 +320,7 @@ export default class CertificateProvider { return Promise.all(deviceMatchList).then(devices => { const matchingIds = devices.filter(m => m.isMatch).map(m => m.id); if (matchingIds.length == 0) { - throw new RecurringError( - `No matching device found for app: ${appName}`, - ); + throw new Error(`No matching device found for app: ${appName}`); } return matchingIds[0]; }); @@ -409,13 +404,11 @@ export default class CertificateProvider { command: string, ): Promise { if (!user.match(allowedAppNameRegex)) { - return Promise.reject( - new RecurringError(`Disallowed run-as user: ${user}`), - ); + return Promise.reject(new Error(`Disallowed run-as user: ${user}`)); } if (command.match(/[']/)) { return Promise.reject( - new RecurringError(`Disallowed escaping command: ${command}`), + new Error(`Disallowed escaping command: ${command}`), ); } return this.adb @@ -426,14 +419,14 @@ export default class CertificateProvider { .then(buffer => buffer.toString()) .then(output => { if (output.match(appNotDebuggableRegex)) { - const e = new RecurringError( + const e = new Error( `Android app ${user} is not debuggable. To use it with Flipper, add android:debuggable="true" to the application section of AndroidManifest.xml`, ); this.server.emit('error', e); throw e; } if (output.toLowerCase().match(operationNotPermittedRegex)) { - const e = new RecurringError( + const e = new Error( `Your android device (${deviceId}) does not support the adb shell run-as command. We're tracking this at https://github.com/facebook/flipper/issues/92`, ); this.server.emit('error', e); @@ -470,13 +463,13 @@ export default class CertificateProvider { .then(subject => { const matches = subject.trim().match(x509SubjectCNRegex); if (!matches || matches.length < 2) { - throw new RecurringError(`Cannot extract CN from ${subject}`); + throw new Error(`Cannot extract CN from ${subject}`); } return matches[1]; }) .then(appName => { if (!appName.match(allowedAppNameRegex)) { - throw new RecurringError( + throw new Error( `Disallowed app name in CSR: ${appName}. Only alphanumeric characters and '.' allowed.`, ); } diff --git a/src/utils/__tests__/errors.node.js b/src/utils/__tests__/errors.node.js deleted file mode 100644 index 645c3cd67..000000000 --- a/src/utils/__tests__/errors.node.js +++ /dev/null @@ -1,16 +0,0 @@ -/** - * Copyright 2018-present Facebook. - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * @format - */ - -import {RecurringError} from '../errors'; - -test('Check RecurringError toString output', () => { - const error = new RecurringError('something'); - expect(error.toString()).toBe('[RecurringError] something'); - /* $FlowFixMe intentionally coercing it to a string to make sure the correct - method is overridden */ - expect('' + error).toBe('[RecurringError] something'); -}); diff --git a/src/utils/errors.js b/src/utils/errors.js deleted file mode 100644 index 92d28578a..000000000 --- a/src/utils/errors.js +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Copyright 2018-present Facebook. - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * @format - */ - -// Class of errors that may keep repeating but should only be logged once. -export class RecurringError extends Error { - constructor(message: string) { - super(message); - this.name = 'RecurringError'; - } - toString(): string { - return `[${this.name}] ${this.message}`; - } -}