Commit Graph

54 Commits

Author SHA1 Message Date
Pascal Hartig
2d3f56ef8d Fix Flipper lints #0
Summary: Fix all lints from T98764173.

Reviewed By: timur-valiev

Differential Revision: D30767919

fbshipit-source-id: 4b0f4b925e246f4fb41d71a323d51fb0e940869d
2021-09-07 03:57:46 -07:00
Michel Weststrate
5e8c968222 Move devices to server folder
Summary:
This is the first of many diffs that extracts the connection, device, client detection out of the flipper core, to create a reusable flipper-server library that can be used in e.g. flipper-dump.

To keep diffs a little smaller, the current connection logic is first moved to the `server/` directory, and decoupled manually from the rest of the core, before moving it over to a separate package.

This first diffs moves the `comms/`, `devices/` and certificate utilities to the `server` directory.

Further untangling will follow in next diffs

Reviewed By: timur-valiev

Differential Revision: D30246551

fbshipit-source-id: c84259bfb1239119b3267a51b015e30c3c080866
2021-08-12 05:43:43 -07:00
Pascal Hartig
dd536b9d1a Gracefully handle socket closures
Summary:
Got my first auto-created crashbot task. {emoji:1f973}

We do have some pretty granular handling for errors during disconnects but this one has fallen through the cracks.

I'm not 100% sure if this is the right way to handle it which is why I added mweststrate. :)

Reviewed By: mweststrate

Differential Revision: D30218833

fbshipit-source-id: 2b4c9201ee7faf1c278b1cc5268ad2648dc4c820
2021-08-10 16:12:02 -07:00
Lorenzo Blasa
b76ce357eb Addresses an issue with Flipper message identifier
Summary:
This change addresses a problem with using data.id straight into the if statement.

If `data.id` is not undefined but has a value of 0 then the expression evaluates to false.

Right now, that wasn't the intended usage.

Using this change to also export some types from Client.

Reviewed By: passy

Differential Revision: D30069746

fbshipit-source-id: 04a8d161efceadf7a38ee556be70c15b45c11056
2021-08-03 09:30:47 -07:00
Lorenzo Blasa
c1496f621e FlipperClientConnection no longer redefines the RSocket Socket interface
Summary:
FlipperClientConnection used to define an interface which closely matched the Socket interface defined in RSocket.

Presumably it tried to 'decouple' RSocket from consumers of the client socket connection. It also limited the amount of actions that could be triggered on said socket connection.

This diff does two things:
- Renames FlipperClientConnection to ClientConnection.
- Changes the interface so that it no longer matches the RSocket Socket interface. The new interface doesn't use RSocket reactive types either.

As a result, current implementations of the interface will act as a proxy to the RSocket Socket type. The change allows the usage of other implementations such as WebSocket.

Reviewed By: fabiomassimo

Differential Revision: D29934090

fbshipit-source-id: 02742e50cd6e801310698969c81b3bf1ef0fa2a2
2021-07-27 08:30:21 -07:00
Michel Weststrate
5dbd3bd414 Make getFlipperLib generally available, and use it to decouple opening links from Electron
Summary:
This stack reduces our direct dependency on Electron, for example by exposing our own API to open links.

Also exposing `getFlipperLib` as API from `flipper-plugin`, so that these utility methods are available outside plugin contexts as well.

Reviewed By: timur-valiev

Differential Revision: D29661689

fbshipit-source-id: 0c0523326eeb0d9d8fbe3e03c4609327bb53596b
2021-07-15 01:54:20 -07:00
Michel Weststrate
328ba9513c Make flipper messages generally available, remove self inspection infra structure
Summary:
Changelog: Flipper message debugging moved from a separate device to the console tab

This makes message debugging easier accessible, and in production (recently requested at GH). Also it clears up a lot of infra that was created just to make flipper a self recursive inspection device + a separate plugin. While fun, a hardcoded setup is just a bit more simpler (no exception rules and better static verification)

Reviewed By: nikoant

Differential Revision: D29487811

fbshipit-source-id: b412adc3ef5bd831001333443b432b6c0f934a5e
2021-07-01 02:00:08 -07:00
Michel Weststrate
16154e1343 Remove classic plugin infra
Summary:
This removes all code duplication / old plugin infra that isn't needed anymore when all plugin run on the Sandy plugin infra structure.

The diff is quite large, but the minimal one that passes tests and compiles. Existing tests are preserved by wrapping all remaining tests in `wrapSandy` for classic plugins where needed

Reviewed By: passy

Differential Revision: D29394738

fbshipit-source-id: 1315fabd9f048576aed15ed5f1cb6414d5fdbd40
2021-06-30 10:42:32 -07:00
Anton Nikolaev
fd9b5cc94d Use set instead of array to keep list of supported plugins per client
Summary: Use set instead of array to keep list of supported plugins per client. It is not used as array anyway, in most places it is used to determine whether a plugin is supported or not and it's much faster to use set for that.

Reviewed By: priteshrnandgaonkar

Differential Revision: D29200673

fbshipit-source-id: 5f3c404a1a668c153867d7c1b6c223941f0b3b36
2021-06-29 13:02:05 -07:00
Michel Weststrate
603cd08767 Stop printing false positive debug message
Summary: Per title. Deserves a more coherent solution.

Reviewed By: passy

Differential Revision: D29296206

fbshipit-source-id: b8547ebc1f99c24282f79f4b98a8a5fa1d1947cd
2021-06-22 05:20:44 -07:00
Michel Weststrate
07199323d1 Remove Newsfeed Story Inspector and native plugin mechanism
Summary: Changelog: The 'nativeplugins' on Android are no longer supported

Reviewed By: jknoxville

Differential Revision: D29163281

fbshipit-source-id: fb4032f240fc306608fe57479f3124d4e7a3400f
2021-06-21 05:39:26 -07:00
Michel Weststrate
c83bd7900f Deprecate Native plugins
Reviewed By: fabiomassimo

Differential Revision: D29163280

fbshipit-source-id: 368e966bf86cc0de1ee0125a6b1166811ef6d2f0
2021-06-17 03:17:54 -07:00
Michel Weststrate
ac0e28a2b1 Introduce some warnings if plugins send to much data
Reviewed By: passy

Differential Revision: D28253456

fbshipit-source-id: 00259076a86cf53ee80282c798c229460d7e4633
2021-05-06 08:23:23 -07:00
Michel Weststrate
699343a9ca Fix bug causing archived devices trying to connect
Reviewed By: nikoant

Differential Revision: D28064540

fbshipit-source-id: 43f05c4348a33e9633751bb9f69cd8d17ddd13c4
2021-04-29 12:12:57 -07:00
Pascal Hartig
c007d74af9 Handle RSocket disconnects gracefully
Summary:
Tiny change, hours of debugging. Big thanks to jknoxville for
answering all my stupid questions.

Our number one "error" right now by users affected is

```
Unhandled Promise Rejection:
Error: RSocketTcpClient:
Socket closed unexpectedly.
```

Of course no stacktrace, because JavaScript and Promises.

The underlying problem is that RSocket keeps track of recently used
"receivers" which is every method that has sent a request. When someone disconnects their device or closes an emulator/simulator, the socket's `end` event is treated the same way any other connection error would be: a85a4db041/packages/rsocket-tcp-client/src/RSocketTcpClient.js (L74)

This then causes "errors" like these to appear:
{F609810187}

This could previously be handled in plugin-code by wrapping all `client.call()` invocations in try/catch, which nobody did, meaning we'd get rejected promises everywhere. We can instead handle this centrally and properly disconnect the connection.

Changelog: Severed RSocket connections are no longer treated as an error in plugin code

Reviewed By: jknoxville

Differential Revision: D27910514

fbshipit-source-id: ea9c0726ab0e959b0eb4a5fca67ddaa04a6f1d14
2021-04-22 07:45:33 -07:00
Michel Weststrate
87c5fab607 Convert crash reporter plugin to Sandy (non UI only)
Summary:
This diff converts the CrashReporter plugin to Sandy. The main driver is that it allows us to fix the connection management of logs in a next diff.

There are few changes to highlight:
* A bunch of the old unit tests are removed, as they primarily verified that persistedState abstraction works, a concept that doesn't exist anymore with Sandy (as a result all the logic in error handling and crash reporter plugin has become a lot more trivial as well)
* Added additional unit tests to verify that the integration with notifications from Sandy, and the integration of crashes in combination with CrashReporter plugin works (this wasn't the case before)
* Plugin errors were always suppressed in production builds of Flipper. However, that makes error reporting pretty pointless in the first place, so enabled it by default, but made it a setting in case this results in too many errors suddenly.
* The integration with clicking OS crash notification -> bringing the user to a sensible place _doesn't_ work, but it didn't work before this diff either, so will address that later
* This doesn't upgrade the Crash reporter UI to sandy yet, will do that later in a separate diff

Changelog: Crash reporter will now report errors triggered from the device / client plugins by default. This can be disabled in settings.

Reviewed By: priteshrnandgaonkar

Differential Revision: D27044507

fbshipit-source-id: 8233798f5cce668d61460c948c24bdf92ed7c834
2021-03-16 15:03:48 -07:00
Michel Weststrate
2ca52f81d2 Introduce pluginKey
Summary: Exposed the `pluginKey` to sandy plugins (which we will use later for storing table preferences per plugin). And a little moving code around because circular deps problem roared its ugly head again.

Reviewed By: nikoant

Differential Revision: D27009721

fbshipit-source-id: 7ad29e72ff8028c9daae270c4749b657bd8ff049
2021-03-16 15:03:47 -07:00
Michel Weststrate
5c3a8742ef Introduce context menu
Summary:
Introduced a context menu for DataTable with some default options. Opted to put it under a visible hovered dropdown instead of on right-click, since this better alings with Ant's design guides (we don't have context clicks anywhere else I think), but if it isn't convincing we can still change it.

Included some default actions, to set up quick filters, and to copy values. For copying rows, implemented it to by default take the JSON from a row, rather than space separated values like in ManagedTable, as many existing plugins customize the onCopy handler to just do that, so it seemed like a better default since it is a richer format. If there are good use cases for the previous behavior, we'll probably find out after the old release :)

Introduced utility to copy text to clipboard in FlipperLib, but decoupled it from Electron.

Didn't include multi select yet, that will be done in a next diff.

Reviewed By: nikoant

Differential Revision: D26513161

fbshipit-source-id: b2b1b20b0a6f4ada9de2566bf6b02171f722c4aa
2021-03-16 15:03:44 -07:00
Anton Nikolaev
fa3ff83595 Rename star/unstar actions to enable/disable/switch
Summary:
Renamed actions "star" and "unstar" everywhere to "enable", "disable" and "switch". The logic behind original "star" action changed significantly, so this rename just makes everything much clearer.

Please note that as a part of rename persisted state fields "userStarredPlugins" and "userStarredDevicePlugins" were renamed. I've added a "redux-persist" migration for seamless transition.

Reviewed By: passy

Differential Revision: D26606459

fbshipit-source-id: 83ad475f9b0231194701c40a2cdbda36f02c3d10
2021-02-24 05:30:57 -08:00
Michel Weststrate
54f9ff260b Show quick notification if plugin interaction fails
Summary:
Per title, if a plugin makes a client call, show a quick notif so that the user knows why his plugin interactions aren't responding in case the plugin doesn't use `isConnected` guards.

This might turn out to be too spammy, but I think it should be ok.

Reviewed By: nikoant

Differential Revision: D26277099

fbshipit-source-id: bd555ea84acae6189ec7f8fff0fc0a088dbc5337
2021-02-09 04:16:27 -08:00
Michel Weststrate
1bb1cae167 Don't send messages to disconnected clients. Make exportPersistedState compatible with disconnected devices.
Summary:
This diff addresses two problems:

1. Since clients plugins can be active beyond having a connection, we have to make it possible for plugin authors to check if they are connected before they make a call.
2. if there is a custom `exportPersistedState`, plugins should be able to skip making calls if the device has disconnected.

Introducing this change makes it possible to interact with a reasonable level with disconnected clients, and makes it possible to create Flipper traces for disconnected clients.

Note that both items were already problems before supporting offline clients; as there can be a noticeable delay between disconnecting and Flipper detecting that (i've seen up to 30 secs). What happend previously in those cases is that the export would simply hang, as would other user interactions, as loosing the connection in the middle of a process would cause the promise chains to be neither rejected or resolved, which is pretty iffy.

Before this diff, trying to export a disconnected device would hang forever like:

{F369600601}

Reviewed By: nikoant

Differential Revision: D26250895

fbshipit-source-id: 177624a116883c3cba14390cd0fe164e243bb97c
2021-02-09 04:16:26 -08:00
Michel Weststrate
7e1bf0f58b Preserve client state after disconnect
Summary:
This diff introduces support for keeping clients around after they have disconnected. This is a pretty important debugging improvement, that will allow inspecting a device / app after it crashed for example.

With this diff, the current client is just kept around until it connects again, instead of throwing clients immediately away if they disconnect. After this change, ArchivedClients will only be created by imports / exports, and no longer by disconnects. Initially I played with improving the creation of archived devices, by migrating all plugin state over from the original client to the archive, but I discovered that is very prone, as it would be a lot of pointer redistribution (plugins would point to a different client / device etc). While in contrast, disconnected clients is already an existing concept in Flipper, so reusing that keeps all the changes relatively simple.

Note that we could potentially still reuse old clients around after reconnected, but it would become much harder to reason about how plugins would behave if they missed updates for a while, so throwing away the device / clients and starting with a fresh slate sounds safer. So I figured that chance to be too risky for now, but would probably be good follow up work.

Issues with import / export, UX, and making calls to to a disconnected client will be addressed in follow up diffs

Changelog: Clients will retain their state after being disconnected, until they reconnect again

Reviewed By: nikoant

Differential Revision: D26224677

fbshipit-source-id: feb9d241df2304341c2847fe7fd751ac54c045f6
2021-02-09 04:16:24 -08:00
Michel Weststrate
12e59afdc6 Remove non-Sandy UI
Summary:
This removes the Non-Sandy UI from the Flipper codebase. It is a pretty rough scan for unused components, over time when converting more advanced components to Ant design probably even more code can be removed.

Partially used `npx ts-purge` to reveal never imported source files.

Changelog: It is no longer possible to opt out of the new Sandy UI

Reviewed By: jknoxville

Differential Revision: D25825282

fbshipit-source-id: 9041dbc7e03bce0760c9a0a34f1877851b5f06cf
2021-01-25 04:48:45 -08:00
Michel Weststrate
7a1717fa87 Clean up App duplication
Summary: fb/App didn't really have a difference with fb-stubs/App, except that one checks for employee presence. However, since that is already controlled by config.checkFbEmployee, de-duped this to a single `<AppWrapper>` component, making the code a bit more easy to follow

Reviewed By: priteshrnandgaonkar

Differential Revision: D25824521

fbshipit-source-id: 8e16f0b29ec5d12475eaf14acd9dbc7df91191a2
2021-01-18 06:48:03 -08:00
Anton Nikolaev
31ace3bd09 Do not show warnings for unhandled messages
Summary: Flipper receives a lot of messages which cannot be handled when plugins unbundled. Showing them as warnings is un-actionable with such big amount, so I disabled them.

Reviewed By: mweststrate

Differential Revision: D25557790

fbshipit-source-id: 1a0bc3df2848b194b72ea6d775ffabe416672fee
2020-12-15 09:31:58 -08:00
Michel Weststrate
25158416ce Freeze incoming data
Summary:
This diff will enabling freezing (making immutable) of all data we receive from the device. This prevents dev mistakes causing components not to update or logic being hard to reason about. Also this makes Immer's `produce` faster in the typical case

Since this is potentially a risky change, that might break existing plugin logic, it has been put behind a GK https://www.internalfb.com/intern/gatekeeper/projects/flipper_frozen_data/

I did some quick exploratory testing on all plugins available for Facebook iOS / Android, and the only plugin that caused trouble was Fresco, which is fixed in this diff as well.

Reviewed By: nikoant

Differential Revision: D25055056

fbshipit-source-id: 8525511f4a8a0221740a6e1371ce7f2b757a203e
2020-11-18 08:51:15 -08:00
Michel Weststrate
2e5b52d247 batch for more efficient message processing
Summary: `unstablebatched_updates` should be used whenever a non-react originating event might affect multiple components, to make sure that React batches them optimally. Applied it to the most import events that handle incoming device events

Reviewed By: nikoant

Differential Revision: D25052937

fbshipit-source-id: b2c783fb9c43be371553db39969280f9d7c3e260
2020-11-18 08:51:15 -08:00
Michel Weststrate
45db64f0d0 Make sure that limited top-level exports are exposed from flipper-plugin
Summary: This prefixes APIs of `flipper-plugin`, that are used by Flipper, but should not be used by plugins directly, with `_`. Also added tests to make sure we are always intentional when extending the exposed APIs

Reviewed By: passy

Differential Revision: D24991700

fbshipit-source-id: ed3700efa188fca7f5a14d5c68250598cf011e42
2020-11-16 13:10:33 -08:00
Michel Weststrate
b66f452271 Keep Navigation plugin alive even when disabled
Summary:
Navigation plugin is a special cause that will remain connected and process messages directly even when disabled, this is to make sure the bookmarks feature keeps working even when the plugin is not enabled.

Changelog: [Sandy][Navigation] on Android, the currently active deeplink of the application will now be shown in the sidebar

Reviewed By: jknoxville

Differential Revision: D24890375

fbshipit-source-id: eb5e4141740e0436396cea5a7aae24337f2e903e
2020-11-12 04:17:29 -08:00
Michel Weststrate
9b4e7e873c Make client -> device connection synchronous
Summary:
devices not always being readily available is causes a lot of complication in the api,
figured to resolve devices first before construction clients,
since clients not attached to a device are shown uncategorized anyway, making them practically un-interactable.
For more background info, see following chat.

{F344388883}

This diff will make it possible to only expose a synchronous api in Sandy

n.b. didn't update Navigation plugin, as that is done in a next diff

Reviewed By: jknoxville

Differential Revision: D24858332

fbshipit-source-id: 8339f831fbbc9c219add56a199364fde67adafc7
2020-11-11 07:58:55 -08:00
Anton Nikolaev
bc39e72fd9 Automatically select plugin after auto-update
Summary: Select reloaded plugin automatically

Reviewed By: mweststrate

Differential Revision: D23987545

fbshipit-source-id: bf54ca54e4aa46b987d57e76fbc4f3e9e5fc336e
2020-09-29 04:26:25 -07:00
Anton Nikolaev
0982dc06a0 Reload single plugin on auto-update
Summary: Implemented a way for re-loading single plugin on auto-update. This make it possible to apply update without full Flipper restart.

Reviewed By: mweststrate

Differential Revision: D23729972

fbshipit-source-id: ed30f7cde5a0537945db0b5bb6969ae8fde42cb6
2020-09-28 02:51:47 -07:00
Michel Weststrate
9c202a4a10 Introduce menu entry support
Summary:
[interesting] since it shows how Flipper APIs are exposed through sandy. However, the next diff is a much simpler example of that

This diff adds support for adding menu entries for sandy plugin (renamed keyboard actions to menus, as it always creates a menu entry, but not necessarily a keyboard shortcut)

```

  client.addMenuEntry(
    // custom entry
    {
      label: 'Reset Selection',
      topLevelMenu: 'Edit',
      handler: () => {
        selectedID.set(null);
      },
    },
    // based on built-in action (sets standard label, shortcut)
    {
      action: 'createPaste',
      handler: () => {
        console.log('creating paste');
      },
    },
  );
```

Most of this diff is introducing the concept of FlipperUtils, a set of static Flipper methods (not related to a device or client) that can be used from Sandy. This will for example be used to implement things as `createPaste` as well

Reviewed By: nikoant

Differential Revision: D22766990

fbshipit-source-id: ce90af3b700e6c3d9a779a3bab4673ba356f3933
2020-08-04 07:47:14 -07:00
Michel Weststrate
489dd1521e Make sure Sandy Devices Plugins are loaded in Flipper devices
Summary:
This diff makes sure that devices will actually instantiate applicable sandy device plugins. Similar to how client plugins are owned by Client, device plugins are directly owned by BaseDevice, which significantly simplifies life cycle management and doesn't dispatch updates to all Redux connect components whenever something irrelevant changes.

Also made sure `device.teardown()` is called. That API already existed, but wasn't used or implemented before.

Updated Flipper test utils to support testing device plugins as well (both Sandy and classic ones)

Reviewed By: passy, nikoant

Differential Revision: D22693929

fbshipit-source-id: 73b2b8666ef7a0e748ea89360db84734d37eb5be
2020-08-04 07:08:31 -07:00
Timur Valiev
d28e763cca Self inspection
Summary:
let's finally inspect flipper with flipper!
Here we have:
1) a self inspection client which implements FlipperClient interface from js sdk and FlipperClientConnection. It links back and front parts of self inspection
2) simple plugin (UI) to show messages
3) back part of that plugin - it sends all received messages to UI part via client
4) we initialize self inspection for dev builds only

P. S. filesystem dependency will be replaced with npm one before I ship it (need to publish to npm first)

Reviewed By: mweststrate

Differential Revision: D22524533

fbshipit-source-id: 5c77e2f7b50e24ff7314e791a4dfe3c349dccdee
2020-07-17 04:54:44 -07:00
Michel Weststrate
44f99eb304 Support export data
Summary:
Made Sandy plugins part of the export / import logic

Note that in the export format there is now a `pluginStates2` field. It could have been put in `pluginStates` as well, but I started with that and it felt very weird, as the `pluginStates` are supposed to reflect what is living in the Redux stores, where sandy state doesn't live in the Redux store (not directly at least). Trying to store it in the same field made things overcomplicated and confusing.

Reviewed By: jknoxville

Differential Revision: D22432769

fbshipit-source-id: d9858e561fad4fadbf0f90858874ed444e3a836c
2020-07-14 09:06:59 -07:00
Michel Weststrate
952e929699 Fix disabling a plugin nog clearing the message queue
Summary:
While writing unit tests discovered a bug that disabling a plugin doesn't guarantee cleaning the messagequeues (both the buffer in client and the messagequeue reducer).

Fixed that. That was thanks to @#%@#$@#%@ Redux a lot harder than it should be; as 'STAR_PLUGIN' reasons about a plugin + app name, while the message queue reducer would need to deduct the plugin keys from that, but it can't because that mapping is stored in the connections reducers. So I moved the `STAR_PLUGIN` action handling to the root reducer, sot that it can reason about the state of multiple reducers, which looked like the least of all evils. For more ranting about that and possible alternative solutions: https://twitter.com/mweststrate/status/1277556309706117122

Reviewed By: nikoant

Differential Revision: D22284043

fbshipit-source-id: 35d0a8ba3a21a5959d2bb6ef17da3ff5077f48fd
2020-07-01 09:12:36 -07:00
Michel Weststrate
bb0c8e0df0 Support receiving messages in Sandy plugins
Summary: This diffs adds the capability to listen to messages in Sandy plugins. Although API wise it looks more like the old `this.subscribe`, semantically it behaves like the `persistedStateReducer`; messages are queued if the plugin is enabled but not active.

Reviewed By: nikoant

Differential Revision: D22282711

fbshipit-source-id: 885faa702fe779ac8d593c1d224b2be13e688d47
2020-07-01 09:12:36 -07:00
Michel Weststrate
df6a8cd031 Provide initial plugin test infra for plugin devs
Summary:
This sets up the initial infra that is to be used by plugin devs to test plugins.

There is not much yet to see, as there is no state or message sending yet. But at least the life cycle of plugins can be test, things are strongly typed and everything is in the place where it should be :)

N.b. the import difference with these utils and the createFlipperMock utilities in Flipper are

1. this testing infra is entirely inside flipper-plugin package, so that plugin devs don't need flipper as a dependency
2. this testing infra doesn't provide abstractions for plugin / device / client switching; it tests plugins purely in isolation of the rest of the world (except for firing `onConnect` / `onDisconnect` which is normally the effect of switching plugins)

Reviewed By: nikoant

Differential Revision: D22255262

fbshipit-source-id: b94ccbab720d2b49428a646aed3c55af71a5bc80
2020-07-01 09:12:36 -07:00
Michel Weststrate
bde112bf85 Introduce onConnect / onDisconnect hooks
Summary:
Introduced hooks that are called whenever the plugin is connected / disconnected to it's counter part on the device.

There is some logic duplication between `PluginContainer` for old plugins, and `PluginRenderer` for new plugins, mostly caused by the fact that those lifecycles are triggered from the UI rather than from the reducers, but I figured refactoring that to be too risky.

Reviewed By: jknoxville

Differential Revision: D22232337

fbshipit-source-id: a384c45731a4c8d9b8b532a83e2becf49ce807c2
2020-07-01 09:12:36 -07:00
Michel Weststrate
04a29315e2 Remove instanceof checks
Summary: Replaced `instanceof` checks with `isSandyPlugin` utility. That is cleaner to read and makes it easier to find places where we make exceptions for Sandy plugins

Reviewed By: jknoxville

Differential Revision: D22206707

fbshipit-source-id: b44a1b585424f3b9bf0d7ce200c34107f03ed55e
2020-07-01 09:12:35 -07:00
Michel Weststrate
1dc9e899b8 Make sure Sandy plugis can be initialized
Summary:
This diff makes sure sandy plugins are initialized.

Sandy plugins are stored directly in the client for two reasons
1. we want to decouple any plugin state updates from our central redux store, as redux is particularly bad in handling high frequency updates.
2. The lifecycle of a plugin is now bound to the lifecycle of a client. This means that we don't need 'persistedStore' to make sure state is preserved; that is now the default. Switching plugins will no longer reinitialize them (but only run specific hooks, see later diffs).
3. PersistedState will be introduced for sandy plugins as well later, but primarily for import / export / debug reasons.

A significant difference with the current persistent state, is that if a client crashes and reconnects, plugins will loose their state. We can prevent this (again, since state persisting will be reintroduced), but I'm not sure we need that for the specific reconnect scenario. Because
1. we should fix reconnecting clients anyway, and from stats it looks to happen less already
2. reconnects are usually caused by plugins that aggregate a lot of data and get slower over time. Restoring old state also restores those unstabilites.

For the overview bringing back the archi picture of earlier diff:
{F241508042}

Also fixed a bug where enabling background plugins didn't enable them on all devices with that app.

Reviewed By: jknoxville

Differential Revision: D22186276

fbshipit-source-id: 3fd42b577f86920e5280aa8cce1a0bc4d5564ed9
2020-07-01 09:12:35 -07:00
Michel Weststrate
1029a6c97c Introduce types for Sandy plugins through code base
Summary:
So far there were 2 types of plugins: `FlipperPlugin` and `FlipperDevicePlugin`. This introduces a third kind: `SandyPluginDefinition`.

Unlike with the old plugins, the export of the module is not directly exposed as the plugin definition. Rather, we use class `SandyPluginDefinition` (instance) that holds a loaded definition and its meta data separately (`PluginDetails`). This means that we don't have to mix in and mutate loaded definitions, and that for unit tests we can avoid needing to provide a bunch of meta data. This also prevents a bunch of meta data existing on two places: on the loaded classes as static fields, and in the meta data field of the loaded class as well. Finally, we can now freely extends the `PluginDetails` interface in flipper, without needing to store it on the loaded classes and we are sure that no naming conflicts are caused by this in the future.

For compatibility with the existing code base, common fields are delegated from the `SandyPluginDefinition` class to the meta data.

Also cleaned up types around plugins a little bit and removed some unnecessary casts.

For all features that reason about plugins in general (such as exports), sandy plugins are ignored for now.

`SandyPluginInstance` is worked out in further diffs

The `instanceof` calls are replaced by a utility function in later diffs.

{F241363645}

Reviewed By: jknoxville

Differential Revision: D22091432

fbshipit-source-id: 3aa6b12fda5925268913779f3c3c9e84494438f8
2020-07-01 09:12:35 -07:00
Michel Weststrate
e31ddbc648 Debounce receiving messages
Summary:
See previous diff.

Achieves the same optimization as in the mentioned diff, but this time by only debouncing the messages as they arrive over the socket, and not the state updates caused by Redux directly. This means that plugin rendering won't be debounced anymore until we address this more fundamentally.

With this change there is a double level buffering:

1. A buffer that stores all incoming messages (that are not replies to requests)
2. A buffer that stores messages we are interested in in the plugin queue, unless the plugin is active (this we already had).

This still fixes the issue that too chatty plugins cause to many updates foreground plugin (the problem we tried to fix originally), without debouncing plugin rendering if it is needed to update for any other reason.

Another nice benefit is that previously every received message would trigger a store update in Redux which would cause all connected components to evaluate their subscriptions (and then bail out in the typical case). Now we will only update the redux store every 200 ms.

Changelog: Foreground plugins will burn less CPU when they're very chatty

Reviewed By: jknoxville

Differential Revision: D21858849

fbshipit-source-id: c72352e569a8a803bbedffb71b17b11fcefee043
2020-06-03 06:54:15 -07:00
Michel Weststrate
dba480ce74 Preserve Navigation plugin as background plugin eevn when disabled
Summary:
This diff fixes an issue where we don't want to have the Navigation plugin be disabled as background plugins like all other plugins, so that the breadcrumb navigation keeps working.

Yet another hack concerning the super useful Navigation plugin. On a positive note, since connection management for background is not entirely managed by the Desktop and not the native said, these kind of exceptions are fairly easy to make :)

Reviewed By: passy

Differential Revision: D21089537

fbshipit-source-id: 209954ff35c95e066fe688a60ad46ccfc3835c44
2020-04-27 09:46:14 -07:00
Michel Weststrate
b9c3d99f44 Stop connecting disabled background plugins
Summary:
Background for this diff: https://fb.quip.com/KqEfAlKYlgme

Some plugins don't respect that stuff (livefeed and graphql), but for others it seems to work fine.

This is just a PoC, there are some present bugs concerning the combination of selecting and bg plugins

Questions to investigate:

- [x] make sure that LiveFeed and GraphQL disconnect properly. There might be more plugins that need that
- [x] verifiy that we don't loose one of the original goals of background plugins, e.g. QPL collecting and sending data from device start. Does this still work as intended after this change?
- [x] how can we observe / measure improvements? Are dev builds more responsive after this? Is the layout inspector smoother for example because no QPL plugins are interweaved?
- [x] how is forward and backward compatibility?
   - If Flipper is updated, but device not: No change I think, as getBackgroundPlugins() will return an empty set, and background plugins are initiated as usual, so old behavior
  - If device is updated, but Flipper not, background plugins won't be started until they are selected. This is a degradation, but hopefully explainable.
- [x] Verify QPL buffer is not unbounded
- [x] Share architecutre changes with team

For Graphql updates: D20943455

Added runtime stats to monitor network traffic (sadly had to redo that since scuba couldn't handle the data format used at first, so probably will hold of landing this diff a week to make sure we can see some effects)

Follow up work:

[x] wait until we released the stat tracking before we release this, to be able to measure the effect?
[x] make sure graphql fix lands
[ ] use side effects abstraction
[ ] fix other background plugins (android only) or fix it in a generic way:

{F234394286}

Changelog: Background plugins will no longer receive a Flipper connection if they are disabled. This should significantly reduce the overall load of Flipper both on the device and desktop when unused plugins are disabled used, which could otherwise generate 10MB/s of network traffic certain scenarios. All plugins *should* be able to handle to this gracefully, but since this is quite a fundamental change, reach out to the Flipper team when in doubt!

Reviewed By: jknoxville

Differential Revision: D20942453

fbshipit-source-id: b699199cb95c1b3e4c36e026b6dfaee7d1652e1f
2020-04-27 09:46:13 -07:00
Michel Weststrate
a5e5ed5b7b Added some additional error handlers to get a better grip on connection issues
Summary: I think it is good to have error handlers where-ever possible, so that there is historical data in our monitoring when we need it, and can track from which point of the code it originates :)

Reviewed By: jknoxville

Differential Revision: D21040059

fbshipit-source-id: 1c07fbfa65379739554bc98f83761ae97870ba82
2020-04-17 05:02:34 -07:00
Michel Weststrate
bc6165bbfe Measure how many bytes are received per plugin
Summary:
Measure how many byte we receive per plugin, and add this to the plugin stats that are collected

Will add a graph to the flipper dashboard, and probably a small visualization in a next diff as well.

Reviewed By: priteshrnandgaonkar

Differential Revision: D20917583

fbshipit-source-id: bb341531ecf8492080af82c56e73c0ec608f7b36
2020-04-08 13:05:59 -07:00
Michel Weststrate
f803cb3cb1 Avoid dropping plugin messages if the connected device is not found
Summary:
Before we dropped all messages if the client connected before the device has been registered. Which happens typically on iOS (it can actually take very long, will investigate that as well, but not in this diff).
However, we don't need to wait for the device to process messages, we just need its serial, which we already know (given that we use that query id to find the matching device in the first place).

I think this will greatly improved perceived stability for iOS

Reviewed By: jknoxville

Differential Revision: D20734892

fbshipit-source-id: f98e8d31558ef606b9a8287e03fc41ab6c3a087d
2020-03-30 07:58:40 -07:00
Michel Weststrate
8fa4b5ccb2 Update existing subscriptions to use sideEffect
Summary: See previous two diffs, this applies the abstraction to our code base

Reviewed By: passy

Differential Revision: D20679687

fbshipit-source-id: 05e340dca3f832971783a844a78d1ffd553ff9d2
2020-03-27 04:42:12 -07:00