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:
Changelog: CPU plugin will no longer show up for archived devices
CPU plugin did show up on imported devices, but would always result in exceptions as no `adb` connection is available for them.
Reviewed By: jknoxville
Differential Revision: D26249575
fbshipit-source-id: c4fa7b3fec895f9c4ab9e31dce2f61fb23e9195b
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: Crash reporter generated React errors because the different lines in a stack trace are not per definition unique
Reviewed By: jknoxville
Differential Revision: D26201941
fbshipit-source-id: 084e0157b1fa3b8216d35f62f0dc17b0d6414b8e
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:
It had this scary warning at the top that the TS version was
too new for the linters.
Reviewed By: nikoant
Differential Revision: D26202273
fbshipit-source-id: df03d6bf176da00ee37b027679b9f258ab123395
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:
Provide a more robust technique for exporting the mocks to a file.
The current technique for exporting mocks to a file seems to fail under some scenarios. The correct format is an array of objects:
```
[
{
"requestUrl": "https://api.github.com/repos/facebook/yoga",
"requestMethod": "GET",
```
However, the following format is sometimes exported instead:
```
{
"10": {
"requestUrl": "https://demo9512366.mockable.io/SonarPost",
```
Using `Object.values` provides a more robust technique that has been successful during subsequent testing.
## Changelog
Network Plugin - new technique for exporting mocks
Pull Request resolved: https://github.com/facebook/flipper/pull/1872
Test Plan:
Create mocks in the network plugin
Export mocks
Manually examine file for correct format
Import mocks and verify that the file has been imported correctly
Reviewed By: nikoant
Differential Revision: D26172954
Pulled By: mweststrate
fbshipit-source-id: bdfa3ba7dfe656f30ef17df001fc83dd8ea18ece
Summary:
Bumps [types/ws](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/ws) from 7.2.7 to 7.4.0.
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a href="https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/ws">compare view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `dependabot rebase` will rebase this PR
- `dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `dependabot merge` will merge this PR after your CI passes on it
- `dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `dependabot cancel merge` will cancel a previously requested merge and block automerging
- `dependabot reopen` will reopen this PR if it is closed
- `dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
</details>
Pull Request resolved: https://github.com/facebook/flipper/pull/1869
Reviewed By: mweststrate
Differential Revision: D26149508
Pulled By: passy
fbshipit-source-id: 6bc9360d297bb9285e3c7515b4731f5c3478c97b
Summary:
Continue with cleanup of Network plugin mock screens. This mostly consists of replacing FlexColumn, FlexRow and FlexBox with equivalent Sandy components.
Here is the new screen:
- replace text buttons with Button
- add NUX info to "Copy Highlighted Calls" button
- add message when no calls have been highlighted
- in routes list remove padding on line items
Here is the prior screen:
## Changelog
Network Plugin - Additional cleanup of UI on mock screen
Pull Request resolved: https://github.com/facebook/flipper/pull/1873
Test Plan:
Create and modify mocks
Verify that the functionality has not been affected by UI changes
Reviewed By: passy
Differential Revision: D26172915
Pulled By: mweststrate
fbshipit-source-id: f0af143d8509b53585076737832657fb095e75a6
Summary:
Made UI fixes to Network Plugin (mostly to Route screens) to continue migration to the new design framework. This consisted mostly of replacing FlexColumn and FlexRow with Layout.Container and Layout.Horizontal.
Also, contains some cosmetic changes to "Mock Network Response" page.
Here is the screenshot with UI changes:
This was the old screen for comparison:
## Changelog
Network Plugin - UI changes to continue migration to Sandy design framework
Pull Request resolved: https://github.com/facebook/flipper/pull/1864
Test Plan: Manual testing to ensure that all data still displayed with new UI changes (especially the Data and Headers info in the "Route Info" section)
Reviewed By: passy
Differential Revision: D26125656
Pulled By: mweststrate
fbshipit-source-id: a25104274ed25788e5c0738ac0a9609f2cead751
Summary: Found out about this today.
Reviewed By: mweststrate
Differential Revision: D26129626
fbshipit-source-id: 82aa5d9fa536010b51fc1cf937f521e5ffa88015
Summary: let's make it easier for users to spot possiblity to jump to source code from flipper
Reviewed By: priteshrnandgaonkar
Differential Revision: D26147686
fbshipit-source-id: 6e248c10e73f3b8fcd937d915af6e91db1fe6abe
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:
In the network plugin, add features to import and export routes as described in issue https://github.com/facebook/flipper/issues/1651
Primary use case is that external testers (such as QA teams) would be able to create test data, convert it to mocks and save the mocks to make bug fixes easier for devs.
Here is a screenshot showing location of buttons to perform import/export (and clearing) of mock routes:
Here is another screenshot showing export dialog:
Changelog: [Network] Mock routes can now be imported and exported. Thanks bizzguy!
Pull Request resolved: https://github.com/facebook/flipper/pull/1855
Test Plan:
Performed manual testing
- create new mocks
- export mocks
- clear mocks
- import mocks
- verify that mocks still work by making GET/POST requests in sample app
Performed various permutations of above manual tests, including restarting Flipper at various points to ensure that test plan still worked. Also performed visual inspection of exported files to verify correctness.
Would be very interested in learning how to create automated tests for this functionality.
Reviewed By: passy
Differential Revision: D26072928
Pulled By: mweststrate
fbshipit-source-id: 51bd5e19e78d830b94add850d5dc9b9e45fa6fad
Summary: importing `antd` in a plugin that lives in the Flipper repo will give a int warning. This fixes that, antd is provided by the host package.
Reviewed By: nikoant
Differential Revision: D26073161
fbshipit-source-id: 897357fafce20129f7e12c035ff99cb4870cc814
