Summary:
*Stack summary*: this stack refactors plugin management actions to perform them in a dispatcher rather than in the root reducer (store.tsx) as all of these actions has side effects. To do that, we store requested plugin management actions (install/update/uninstall, star/unstar) in a queue which is then handled by pluginManager dispatcher. This dispatcher then dispatches all required state updates.
*Diff summary*: refactored Flipper mocking helpers to allow testing of plugin commands, and wrote some tests for pluginManager.
Reviewed By: mweststrate
Differential Revision: D26450344
fbshipit-source-id: 0e8414517cc1ad353781dffd7ffb4a5f9a815d38
Summary:
*Stack summary*: this stack refactors plugin management actions to perform them in a dispatcher rather than in the root reducer (store.tsx) as all of these actions has side effects. To do that, we store requested plugin management actions (install/update/uninstall, star/unstar) in a queue which is then handled by pluginManager dispatcher. This dispatcher then dispatches all required state updates.
*Diff summary*: implemented basic plugin action queue processing.
Reviewed By: mweststrate
Differential Revision: D26164945
fbshipit-source-id: 5d8ad9b4d7b1300e92883d24a71da9ca1f85b183
Summary: Found some code duplication causing old devices not entirely to be cleaned up as they should
Reviewed By: nikoant
Differential Revision: D26422665
fbshipit-source-id: f57757f7260ac5de17edd80e9f66cc844d1a6213
Summary:
Changelog: Fixed issue where a Flipper crash would result in an entirely blank screen, rather than a useful error message.
While debugging another issue, discovered that React errors that happen outside a Plugin aren't caught at all, resulting in the infamous gray screen of deaths. This was the case because no error boundary has been set up for our Chrome, and since React 16 the default error handling has becoming rendering blank, rather than freezing. See https://reactjs.org/docs/error-boundaries.html#new-behavior-for-uncaught-errors.
Thanks to ant.design styling this decently was trivial :). But sadly involved a component class since error boundaries are not yet available as hook.
With these changes the errors should also end up more readably in our monitoring.
Reviewed By: nikoant
Differential Revision: D26422666
fbshipit-source-id: 6c0f8611c80a4a5e0d7e61d58afcf5eabe410e57
Summary:
T84629221 shows this is quite common and is not something we want
to log errors for.
Reviewed By: mweststrate
Differential Revision: D26371400
fbshipit-source-id: 748a83d8ad484b0ceb1f3bf7e3a447602ae4c961
Summary:
This is our #1 error and always means there's something off with
the user's setup.
Reviewed By: mweststrate
Differential Revision: D26370974
fbshipit-source-id: 5e2644ce9de8a7d28614a22fab5ce54dab0561d3
Summary:
Adding context everywhere as naked messages are hard to track down
and making common errors like missing Xcode license agreements
a warning + error popup instead.
Changelog: Reduce spamminess of iOS connection warnings
Reviewed By: mweststrate
Differential Revision: D26370235
fbshipit-source-id: b283d297479c2bf76a11b5a3f96a59fad70a4a67
Summary: Noticed in the support group that the imported device label was incorrect. Probably a copy paste error.
Reviewed By: nikoant
Differential Revision: D26368664
fbshipit-source-id: abc2aad4d20a9f44d6f1ac45962ab28863d78f0d
Summary:
I want to use TypesScipt type literals in a next diff (e.g.
```
type Percentage = `${number}%`
```
But to be able to use that typescript, prettier and eslint needed bumps :)
Reviewed By: nikoant
Differential Revision: D26321133
fbshipit-source-id: a4891246ef8c654f324c6daf303c5c4b2f54496e
Summary:
Some minor loose ends from exploratory testing:
- disconnect device logs event listeners if the device disconnects
- show metro if that is still up, even if the app is no longer connected
- hide the options in the support form to record videos / screenshots if the device isn't connected
Reviewed By: nikoant
Differential Revision: D26277100
fbshipit-source-id: 49d0c934d587b226bc25524224efce60b53939e9
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
Summary:
Previously, plugins could relatively safely use `client.call` at any moment in time to fetch some information from / to the client. Except for some raise conditions there was generally speaking a connection available.
With this stack it becomes possible to interact with plugins even after an app (unexpectedly) disconnected, which makes Flipper a lot more versatile, especially when it comes to inspect crashes post mortem. (For more explanation see the second diff in this stack)
However, this means that it is no longer safe to assume there is always a connection available. For that reason `client.isConnected` has been introduced to safeguard against that.
This diff introduces guards on all user interactions that are not super explicitly triggered by the user to avoid a lot of errors being generated. This is mostly in `init()` blocks and implicit user events like selecting or hovering rows.
Explicit user interactions like pressing buttons are not guarded by this diff, as ideally failure to communicate with the client should be communicated back to the user more explicitly rather than failing silently. The next diff will introduce a fallback mechanism where a popup is shown in case those calls aren't guarded.
Fixed a few key warnings while at it.
Reviewed By: nikoant
Differential Revision: D26275604
fbshipit-source-id: 5630066cdd9541e448a6dd1f8a21861b5d751ced
Summary:
Minor code cleanup to avoid future confusion:
- archived: a device that was imported from a Flipper trace, and only has persisted state
- (dis)connected: a real stateful device that might or might not have an active connection
Reviewed By: nikoant
Differential Revision: D26275459
fbshipit-source-id: eba554b37c39711e367c3795ff4456329a303c22
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
Summary:
During testing I noticed that even though plugin queues were flushed, the processed messages didn't end up in the export snapshots. This was caused by holding a ref of an older snapshot of the state
Changelog: Fixed an issue where data that arrived in the background was not part of the generated Flipper export.
Reviewed By: nikoant
Differential Revision: D26250897
fbshipit-source-id: ddd3f5bb19e38a1b13498d03f235bf63858eb8f8
Summary:
Small UX improvement, try to select a newly arriving client if possible, this is nice as it means that disconnecting and connecting will typically end you up in the same app.
Changelog: If a new client connects, Flipper will try to focus on it
Reviewed By: nikoant
Differential Revision: D26250896
fbshipit-source-id: 83d9777a8608cd887d663a6bbe1444d2aa614e95
Summary:
It should be possible to exported disconnected devices, so that flipper traces / support form reports can be created from them. This diff introduces this functionality. Support for plugins with custom export logic is introduced in a later diff.
Issues fixed in this diff:
- don't try to take a screenshot for a disconnected device (this would hang forever)
- device plugins were always exported, regardless whether the user did select them or not
- sandy plugins were never part of exported disconnected clients
- increased the amount of data exported for device logs to ~10 MB. This makes more sense now as the logs will no longer be included in all cases
- fixed issue where are plugins would appear to be enabled after the client disconnected (this bug is the result of some unfortunate naming of `isArchived` vs `isConnected` semantics. Will clean up those names in a later diff.
Changelog: It is now possible to create a Flipper trace for disconnected devices and apps
Reviewed By: nikoant
Differential Revision: D26250894
fbshipit-source-id: 4dd0ec0cb152b1a8f649c31913e80efc25bcc5dd
Summary: UX love for imported and disconnected devices, so that stuff looks better :)
Reviewed By: nikoant
Differential Revision: D26249348
fbshipit-source-id: 70db682ccf0cb73161e136994f5135717f3c6be6
Summary:
Introduced `isConnected` flag on device and plugin client to reflect whether a connection is still available for the plugins, or that they have been disconnected.
Potentially we could expose the (readonly) `connected` state atom for this as well, or an `onDisconnect` event for device pugins, to create a responsive UI, but there might be no need for that, in which case this suffices.
Reviewed By: nikoant
Differential Revision: D26249346
fbshipit-source-id: b8486713fdf2fcd520488ce54f771bd038fd13f8
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
Summary:
This diff stack introduces support for keeping devices and 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.
This feature existed partially before, but only supported Android, and only support plugins with persisted state; as it replace the current device with an archived version of the same device. In practice this didn't work really well, as most plugins would not be available, and all non-persisted state would be lost.
This diff makes sure we can keep devices around after disconnecting, the next one will keep the clients around as well. And explain some code choices in more detail.
Note that `Device.isArchived` was an overloaded term before, and even more now (both representing imported and disconnected devices), will address that in a later diff.
https://github.com/facebook/flipper/issues/1460https://github.com/facebook/flipper/issues/812https://github.com/facebook/flipper/issues/1487
Changelog: iOS and Android devices will preserve their state after being disconnected
Reviewed By: nikoant
Differential Revision: D26224310
fbshipit-source-id: 7dfc93c2a109a51c2880ec212a00463bc8d32041
Summary:
The any type was masking how the log object is actually initialised.
Sorry for the deluge of drive-by diffs. Something more substantial is coming.
Reviewed By: mweststrate
Differential Revision: D26250580
fbshipit-source-id: 5ba3f450ac1a646616868a8fd8b3cb42fb14dcc8
Summary:
There's a bit of an oddity with `idb` that the `stream` parameter is implied even though the docs say otherwise and if you try to use it, it'll give you a strange Python error. That's likely why we never implemented it.
Now, it works just as it does using local tooling.
Reviewed By: mweststrate
Differential Revision: D26228036
fbshipit-source-id: e20cb31167170ba0501e2929ed129305cb9aaf2c
Summary: The "attaching failed" error provides no context and is a handled error, so we don't need to elevate this to a warning. We also see a socket warning on stderr every time we start up logging through simctl, so we don't need to treat that as an error.
Reviewed By: nikoant
Differential Revision: D26228037
fbshipit-source-id: 1938dadd54499462e1fd614c9477f738661c387b
Summary:
VSCode got a new thing where it doesn't save my file because it waits for a formatter or some stuff indefinitely, which is really cool, because it means my diffs end up being incomplete.
So this should have been in D26223274 (642d89213d) but now it's here.
angry_cry
Reviewed By: nikoant
Differential Revision: D26228038
fbshipit-source-id: 98b84179dce4e8e8c71f9196ab78d534327ea301
Summary:
Just some simple memoisation so we limit this particular `remote`
call to one per session.
Reviewed By: mweststrate
Differential Revision: D26223274
fbshipit-source-id: 7a12764758823c52f68fb7075f46caf58affb22f
Summary:
This diff adds gk checks, VPN check and User login check to our plugin.
I added gk checks for Insta and FB apps. There are three kind of GK's. One with FB universe, one with Distillery universe and one with Insta WWW universe. We have API's to check gk with FB and Distillery universe. API to check with Insta WWW universe doesn't exists. There are some technical challenges to build this API. So right now for gks with Insta WWW universe, we just show an notification with info icon. For gks in other universe we aggressively check it when user closes the notif to verify if user assigned himself/herself to it.
In long term we are moving away from gks altogether, we need this UX improvement as it will help users to onboard easily.
See the following flow.
Reviewed By: mweststrate
Differential Revision: D26176996
fbshipit-source-id: 92a931610f9b244c14c6888bb12df936b62edd75
Summary:
These are recoverable, handled errors. We shouldn't treat these
as events that require actions from us. They also have
a tendency of firing a lot.
Reviewed By: nikoant
Differential Revision: D26202358
fbshipit-source-id: 445f3c2bcd0041d5cd773ec04172fdeed9b32222
Summary:
Using our serialization utilities in the previous diff absolute destroyed the performance of serializing the device logs. Investigated a bit, and the root cause is that *every* object serialization would notify the UI.
This is not only pointless, since the UI won't be updated until the next tick anyway, it also is terribly expensive since React has to process and queue all these updates.
Exporting the device logs went down from **2 minutes to a few seconds** with this change. (Still a lot slower than JSON.stringify, but I think the flexibility for plugin devs is worth it).
This change does not only benefit devicelogs plugin, but all existing plugins as well, plugins like GraphQL should now export their data much quicker.
Before (practically all time of serialization is spend in React's setState):
{F366147730}
After (only a spike at the end of the idler tick):
{F366147779}
Reviewed By: priteshrnandgaonkar
Differential Revision: D26146420
fbshipit-source-id: 9bbeccf04701fd044e041956b7bb00f1e0622b63
Summary:
Unlike non-sandy plugins, non-sandy plugins weren't serialized using our serialization utility yet. This diff addresses that, meaning that users don't have to bother about how to serialize maps, sets and dates.
Unlike the old fashioned plugins, the `makeObjectSerialize` utility is used, rather than `serialize`. This normalizes the objects, but doesn't serialize them, which is done at the end of the export data process anyway for the whole tree. This avoids creating a double JSON serialization which is fully of ugly escape characters.
This makes the onImport / onExport definition of the logs plugin nicer.
Also improved the docs.
Reviewed By: nikoant
Differential Revision: D26146421
fbshipit-source-id: 6abfb6ee2e3312e2a13a11832ff103dc62fd844c
Summary:
While creating some other tests, discovered that our current date serialization uses `toString()` serialization, causing the amount of milliseconds to be lost. The serialization (see below) uses less bytes as well since the human readable timezone isn't included. This change only affects serialization and is backward compatible.
```
✓ test serialize and deserializeObject function for non Object input (1 ms)
✕ test makeObjectSerializable and deserializeObject function for Date input (2 ms)
✓ test makeObjectSerializable and deserializeObject function for Map of Sets
✓ test makeObjectSerializable and deserializeObject function for Map, Dates and Set with complex nesting (1 ms)
● test makeObjectSerializable and deserializeObject function for Date input
expect(received).toEqual(expected) // deep equality
Expected: 2021-03-01T10:31:07.205Z
Received: 2021-03-01T10:31:07.000Z
```
Reviewed By: priteshrnandgaonkar
Differential Revision: D26145941
fbshipit-source-id: dfd6607a4199ca46e2075027856138efb88a07f9
Summary:
Logs were stored hardcoded on the Device object first, this diff makes it normal plugin state.
This makes sure that we can use the same abstractions as in all plugins that store large data sets, and that we can leverage the upcoming DataSource abstraction.
Reviewed By: nikoant
Differential Revision: D26127243
fbshipit-source-id: 7c386a615fa7989f35ba0df5b7c1d218d37b57a2
Summary: Per title, this allows for pre-processing data after it is deserialized and before it is stored in the plugin
Reviewed By: nikoant
Differential Revision: D26126423
fbshipit-source-id: bc08a6ab205d2a0d551515563cd85a197595ddb2
Summary:
Sandy plugins can now set up an `onExport` handler to enable customizing the export format of a plugin: `client.onExport(callback: (idler, onStatusMessage) => Promise<state>)`
Import will be done in next diff
Reviewed By: nikoant
Differential Revision: D26124440
fbshipit-source-id: c787c79d929aa8fb484f15a9340d7c87545793cb
Summary: Sandy device plugins weren't exported till now (the only stateful plugin so far was Logs, but logs were stored hardcoded on the device rather than using the plugin export mechanisms). This diff makes sure that SandyDevicePlugins will be exported as well if they are persistable.
Reviewed By: nikoant
Differential Revision: D22724822
fbshipit-source-id: a10354a9c7e02f3e696d0cdda0f2c6be6f5ac61e
Summary:
While trying to change something, discovered we have 3 different mechanisms in our code base to compute active plugins; the plugin list component, support form, and export flipper trace form had all their own, subtly different implementations of computing which plugins are available to the user.
Also removed some hardcoded exceptions for e.g. Logs plugin, which in the next diff and onward will be just a vanilla plugin without special casing
Unified that, which some how went a bit deeper than hoped, trough some hoops in in circular deps. Also unified to use the same testing utils, to avoid some gobbling objects manually together, with resulted in a bunch of unexpected NPEs. Found out that we actually still have unit tests using Flow in the process :-P. Converted one to TS.
Reviewed By: nikoant
Differential Revision: D26103172
fbshipit-source-id: 2fce2577d97d98543cb9312b3d013f24faee43aa
Summary: Found out about this today.
Reviewed By: mweststrate
Differential Revision: D26129626
fbshipit-source-id: 82aa5d9fa536010b51fc1cf937f521e5ffa88015
Summary: Going to make more changes here soon and wanted to apply some small changes first.
Reviewed By: jknoxville
Differential Revision: D26078645
fbshipit-source-id: 3a2bcd593b893160b5a332c858a514ebe89d3f4d
Summary: Plugin metadata format extended to include type of each plugin (client / device) and list of supported devices (android/ios/..., emulator/physical, etc). This will allow to detect plugins supported by device even if they are not installed and only available on Marketplace.
Reviewed By: mweststrate
Differential Revision: D26073531
fbshipit-source-id: e331f1be1af1046cd4220a286a1d52378c26cc53
Summary: This diff fixes the issue when there is an error on checking GK for any of plugins and because of it the entire set of plugins failed to load. Each plugin should be loaded in isolation from others.
Reviewed By: passy
Differential Revision: D26099735
fbshipit-source-id: ba5475f4baf2d06f8922d345c9d401f5b15956ec
Summary:
The support form currently has a search form to select a group, but unless the selection is cleared, it won't show you which groups is actually available, which makes it hard for people to select the right group if they don't know up front.
Since the scale of available groups doesn't justify needing a typeahead, converted it to an ordinary dropdown.
An added benefit is that this allows us to remove a large and complicated component we shouldn't be maintaining ourselves, but rather reuse from Ant.
Reviewed By: nikoant
Differential Revision: D26046131
fbshipit-source-id: f499e5848eec8b961b054104c8e3a01567e2801e
