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

Unify error notifications (#1483)

Browse Source 
Summary:
Note: this is to be stacked upon https://github.com/facebook/flipper/pull/1479

Note: this PR will probably not succeed against FB internal flipper, as I'm pretty sure there are more call sites that need to be updated. So consider this WIP

Currently connection errors are managed in the connection reducers, and are displayed through their own means, the error bar. Showing console.errors is also hooked up to this mechanism in FB internal flipper, but not at all in the OSS version, which means that some connection errors are never shown to the user.

Besides that there is a notification system that is used by for example the crash reporter and plugin updater.

Having effectively (at least) two notifications mechanisms is confusing and error prone. This PR unifies both approaches, and rather than having the connection reducer manage it's own errors, it leverages the more generic notifications reducer. Since, in the previous PR, console errors and warnings have become user facing (even in OSS and production builds, which wasn't the case before), there is no need anymore for a separate error bar.

I left the notifications mechanism itself as-is, but as discussed in the Sandy project the notification screen will probably be overhauled, and the system wide notifications will become in-app notifications.

## Changelog

Pull Request resolved: https://github.com/facebook/flipper/pull/1483

Test Plan: Only updated the unit tests at this point. Manual tests still need to be done.

Reviewed By: passy

Differential Revision: D23220896

Pulled By: mweststrate

fbshipit-source-id: 8ea37cf69ce9605dc232ca90afe9e2f70da26652
This commit is contained in:
Michel Weststrate
2020-08-21 10:05:19 -07:00
committed by Facebook GitHub Bot
parent 76b72f3d77
commit 81eb09e7b0
11 changed files with 133 additions and 435 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
desktop/app/src/dispatcher/androidDevice.tsx
View File

@@ -19,7 +19,7 @@ import which from 'which';
import {promisify} from 'util';
import {ServerPorts} from '../reducers/application';
import {Client as ADBClient} from 'adbkit';
import {addNotification} from '../reducers/notifications';
import {addErrorNotification} from '../reducers/notifications';
function createDevice(
adbClient: ADBClient,
@@ -84,22 +84,14 @@ function createDevice(
const isAuthorizationError = (e?.message as string)?.includes(
'device unauthorized',
);
console.error('Failed to initialize device: ' + device.id, e);
store.dispatch(
addNotification({
client: null,
notification: {
id: 'androidDeviceConnectionError' + device.id,
title: 'Could not connect to ' + device.id,
severity: 'error',
message: `Failed to connect to '${device.id}': ${
isAuthorizationError
? 'make sure to authorize debugging on the phone'
: JSON.stringify(e, null, 2)
}`,
},
pluginId: 'androidDevice',
}),
addErrorNotification(
'Could not connect to ' + device.id,
isAuthorizationError
? 'Make sure to authorize debugging on the phone'
: 'Failed to setup connection',
e,
),
);
}
resolve(undefined); // not ready yet, we will find it in the next tick

17
desktop/app/src/dispatcher/iOSDevice.tsx
View File

@@ -20,6 +20,7 @@ import iosUtil from '../utils/iOSContainerUtility';
import IOSDevice from '../devices/IOSDevice';
import isProduction from '../utils/isProduction';
import {registerDeviceCallbackOnPlugins} from '../utils/onRegisterDevice';
import {addErrorNotification} from '../reducers/notifications';
type iOSSimulatorDevice = {
state: 'Booted' | 'Shutdown' | 'Shutting Down';
@@ -209,18 +210,10 @@ async function checkXcodeVersionMismatch(store: Store) {
);
const runningVersion = match && match.length > 0 ? match[0].trim() : null;
if (runningVersion && runningVersion !== xcodeCLIVersion) {
const errorMessage = `Xcode version mismatch: Simulator is running from "${runningVersion}" while Xcode CLI is "${xcodeCLIVersion}". Running "xcode-select --switch ${runningVersion}" can fix this.`;
store.dispatch({
type: 'SERVER_ERROR',
payload: {
message: errorMessage,
details:
"You might want to run 'sudo xcode-select -s /Applications/Xcode.app/Contents/Developer'",
urgent: true,
},
});
// Fire a console.error as well, so that it gets reported to the backend.
console.error(errorMessage);
const errorMessage = `Xcode version mismatch: Simulator is running from "${runningVersion}" while Xcode CLI is "${xcodeCLIVersion}". Running "xcode-select --switch ${runningVersion}" can fix this. For example: "sudo xcode-select -s /Applications/Xcode.app/Contents/Developer"`;
store.dispatch(
addErrorNotification('Xcode version mismatch', errorMessage),
);
xcodeVersionMismatchFound = true;
break;
}

16
desktop/app/src/dispatcher/metroDevice.tsx
View File

@@ -13,6 +13,7 @@ import {registerDeviceCallbackOnPlugins} from '../utils/onRegisterDevice';
import MetroDevice from '../devices/MetroDevice';
import {ArchivedDevice} from 'flipper';
import http from 'http';
import {addErrorNotification} from '../reducers/notifications';
const METRO_PORT = 8081;
const METRO_HOST = 'localhost';
@@ -129,15 +130,12 @@ export default (store: Store, logger: Logger) => {
const guard = setTimeout(() => {
// Metro is running, but didn't respond to /events endpoint
store.dispatch({
type: 'SERVER_ERROR',
payload: {
message:
"Found a running Metro instance, but couldn't connect to the logs. Probably your React Native version is too old to support Flipper.",
details: `Failed to get a connection to ${METRO_LOGS_ENDPOINT} in a timely fashion`,
urgent: true,
},
});
store.dispatch(
addErrorNotification(
'Failed to connect to Metro',
`Flipper did find a running Metro instance, but couldn't connect to the logs. Probably your React Native version is too old to support Flipper. Cause: Failed to get a connection to ${METRO_LOGS_ENDPOINT} in a timely fashion`,
),
);
registerDevice(undefined, store, logger);
// Note: no scheduleNext, we won't retry until restart
}, 5000);

33
desktop/app/src/dispatcher/server.tsx
View File

@@ -13,6 +13,7 @@ import {Store} from '../reducers/index';
import {Logger} from '../fb-interfaces/Logger';
import Client from '../Client';
import {UninitializedClient} from '../UninitializedClient';
import {addErrorNotification} from '../reducers/notifications';
export default (store: Store, logger: Logger) => {
const server = new Server(logger, store);
@@ -42,17 +43,14 @@ export default (store: Store, logger: Logger) => {
});
server.addListener('error', (err) => {
const message: string =
err.code === 'EADDRINUSE'
? "Couldn't start websocket server. Looks like you have multiple copies of Flipper running."
: err.message || 'Unknown error';
const urgent = err.code === 'EADDRINUSE';
store.dispatch({
type: 'SERVER_ERROR',
payload: {message},
urgent,
});
store.dispatch(
addErrorNotification(
'Failed to start websocket server',
err.code === 'EADDRINUSE'
? "Couldn't start websocket server. Looks like you have multiple copies of Flipper running."
: err.message || 'Unknown error',
),
);
});
server.addListener('start-client-setup', (client: UninitializedClient) => {
@@ -74,11 +72,14 @@ export default (store: Store, logger: Logger) => {
server.addListener(
'client-setup-error',
(payload: {client: UninitializedClient; error: Error}) => {
store.dispatch({
type: 'CLIENT_SETUP_ERROR',
payload: payload,
});
({client, error}: {client: UninitializedClient; error: Error}) => {
store.dispatch(
addErrorNotification(
`Connection to '${client.appName}' on '${client.deviceName}' failed`,
'Failed to start client connection',
error,
),
);
},
);