jens/flipper
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove RecurringError type

Browse Source 
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
This commit is contained in:
John Knox
2019-03-22 06:48:29 -07:00
committed by Facebook Github Bot
parent daf8cfea2a
commit bf9be74ad2
6 changed files with 18 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/dispatcher/iOSDevice.js
View File

@@ -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<Array<IOSDeviceParams>> {
function getActiveDevices(): Promise<Array<IOSDeviceParams>> {
return iosUtil.targets().catch(e => {
console.error(new RecurringError(e.message));
console.error(e.message);
return [];
});
}

11
src/reducers/connections.js
View File

@@ -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,

9
src/server.js
View File

@@ -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',
);
}

27
src/utils/CertificateProvider.js
View File

@@ -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<string> {
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.`,
);
}

16
src/utils/__tests__/errors.node.js
View File

@@ -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');
});

17
src/utils/errors.js
View File

@@ -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}`;
}
}