Summary:
Whenever there is an issue, I always find myself adding these logs.
Instead of keep doing that, reuse the existing FLIPPER_DEBUG_LOG definition as a way of enabling/disabling the verbose logs very easily.
Reviewed By: ivanmisuno
Differential Revision: D49227973
fbshipit-source-id: d053df6668b1dda6f6782e8ef7d844c82945f126
Summary: Set delegate and close inside the operation's queue as to make it safer i.e. all socket related operations are done inside the queue.
Reviewed By: ivanmisuno
Differential Revision: D47124235
fbshipit-source-id: 48b53db1cd47d017a26186a156046ba68fe358b7
Summary:
Never really liked this code. Before this change, calls to connect were blocking.
Because of this, we had to make use of promises and a bit of really not that good-looking code.
So, this change makes connect non-blocking meaning that we make full use of our event handler.
These changes contain:
- CSR is not getting generated after each failed attempt.
- Connect is no longer blocking.
- Do not report events via the handler when explicitly disconnecting.
Reviewed By: jknoxville
Differential Revision: D46853228
fbshipit-source-id: 00e6a9c7c039a756175fe14982959e078d92bacb
Summary:
Create the step as an attempt.
Remove conditionally marking it as success or failure as connect will not be done 'synchronously' moving forward.
:::notes:::
Documentation is for personal use and will be removed next.
Reviewed By: passy
Differential Revision: D46850048
fbshipit-source-id: d6dce961d5cbd767f428e58850d24a433d50ba14
Summary: Not really part of the connect process in this case, so is getting moved closer to where the CSR is generated.
Reviewed By: passy
Differential Revision: D46849966
fbshipit-source-id: b91e2925552cd7fcab67b3d1d7af006f4f8c1431
Summary: This was a temporary variable which is not really needed, so remove.
Reviewed By: passy
Differential Revision: D46849864
fbshipit-source-id: 56fb52f9a80128fb746afcdc4d36225e6d596db2
Summary:
The event handler is a friend type that was mutating the connection manager state.
Instead, just forward the event handling to it.
Then state mutation is consolidated inside the connection manager.
Reviewed By: passy
Differential Revision: D46849769
fbshipit-source-id: 594ab32c8e891564afa94e1be6b93b1dfeffe26f
Summary:
Expose a new API to be used to check if there's an open connection with Flipper Desktop.
Changelog: new FlipperClient isConnected API
Reviewed By: antonk52
Differential Revision: D46841095
fbshipit-source-id: 82a60f52496fb218cb50c6a28d7ffe7225ae23aa
Summary:
This change is the direct results of doing the following:
npx react-native-windows-init --overwrite
Notes: ignore the format warnings below. In this case, the ordering of includes does matter. Missing license is for auto-generated files, so ignore too.
Reviewed By: aigoncharov
Differential Revision: D36775215
fbshipit-source-id: 1cd00ff2bfc258c8505e97dcdbd9cb4365c4acfb
Summary:
folly::dynamic, std::string, implicit constructors and method overloading is not a good combination.
This renames the send method to sendRaw as to avoid issues with existing plugins currently sending string params.
Reviewed By: mweststrate
Differential Revision: D38827539
fbshipit-source-id: 653f62e41ebfbe93d1af25f39c81f6b05bf84cb4
Summary:
For C++, folly::dynamic is used throughout.
On iOS and Android though, Flipper goes through multiple conversions to get to a folly::dynamic only to ultimately obtain a JSON string from it.
Let's take a look at Android:
There are multiple types like FlipperObject, FlipperArray that wrap around a JSONObject.
When data needs to be sent:
1. The JSONObject is asked for its string representation.
2. The string representation is then parsed by folly to construct the folly::dynamic instance.
3. The step above involves an extra boundary cross through JNI.
4. Ultimately, a socket or ws connection does not understand folly::dynamic so we then get a JSON string representation from it.
5. Data is sent.
As described above, for big enough objects, this represents an issue.
So, the idea of this change, is to allow plugins to send a JSON string instead. This will remove a few serialisation/deserialisation steps from the process.
*Note: this API is not currently used by anything so there's no impact to existing plugins.*
Changelog: expose a send method that accept a string as params
Reviewed By: LukeDefeo
Differential Revision: D38741582
fbshipit-source-id: 78e0acd80fc8c97378ee986cbaf377078996ed60
Summary:
^
If Flipper stops, we cannot freely access the client outside of the scheduler.
There may be an scheduled operation that depends on it.
Instead, schedule the stop and wait until it finishes.
Reviewed By: cekkaewnumchai
Differential Revision: D37814820
fbshipit-source-id: 44217b5f623a8d92211a3d72a4a204d674b4bb76
Summary:
This change isolates the usage of folly async from Flipper. Is now self-contained in Flipper Folly schedulers.
Users of Flipper can decide not to use the types defined in that header and implement their own.
NOTE: changes are minimal, we are just replacing direct calls to folly event base with a scheduler which simply relays this on to folly.
Reviewed By: fabiomassimo
Differential Revision: D36626483
fbshipit-source-id: add0241caf4af0aa5c3b5c2e7efc2e725f5400ab
Summary:
This change isolates the usage of folly async from Flipper. Is now self-contained in Flipper Folly schedulers.
Users of Flipper can decide not to use the types defined in that header and implement their own.
NOTE: changes are minimal, we are just replacing direct calls to folly event base with a scheduler which simply relays this on to folly.
Reviewed By: fabiomassimo
Differential Revision: D36052198
fbshipit-source-id: 170d64a324a1f1f100224e2622a59cbac3c8b642
Summary:
Flipper doesn really use library specific exceptions throughout, and that's OK.
Introducing SSLException as a replacement for the existing Folly Async Socket SSL exception.
This exception originally thrown by rsocket. Because we had rsocket and websockets using the same code, websockets were creating and throwing this same exception.
With rsocket gone, we can fully replace the usage of that exception. This is also needed as to decouple Flipper from folly async components.
Reviewed By: fabiomassimo
Differential Revision: D36245624
fbshipit-source-id: f5c97c5efe063280ce95be130008dee7f4e5d788
Summary:
To ensure that no deadlocks take place, it is important that there are no re-entrant calls from within the callbacks or event handlers.
For the most part, this was already the case. Event and message handlers run critical sections into a Folly event scheduler.
The only exception was the sendExpectResponse used during the certificate exchange. Once the response was received, the non-secure socket was disconnected.
The solution was to put that operation in the Folly event scheduler as it should've been from the beginning.
changelog: Certificate signing request response to be processed on the right event loop.
Reviewed By: fabiomassimo
Differential Revision: D35548148
fbshipit-source-id: cea2476ad66137f376acda66cdbc27801c0c47e1
Summary:
^
Note: this is already a working case. The difference is that if we are unable to establish a socket connection, we will not attempt to create one using rsocket.
Changelog: Removes rsocket-fallback for mobile clients
Reviewed By: nikoant
Differential Revision: D33655430
fbshipit-source-id: cb6f752f2d1354ab46d011b1f19c89520e1e7dd3
Summary:
The problem seems to be exclusive to Android emulators.
Bringing the emulator ON from a warm state brings the app from is previous state before shutting down. At this point there may have been a Flipper connection.
Flipper tries to connect and does so successfully replacing the previously, now dead connection.
The problem is that replacing that connection triggers the reconnect cycle as Flipper thinks the connection is dead and hence the reconnect loop initiates.
Changelog: Release an existing client before attempting a secure connection
Reviewed By: passy
Differential Revision: D34080726
fbshipit-source-id: 8185adb492dd4d9255fcea5874ca2e5b7fee0c84
Summary:
Changelog: Fallback to RSockets if cert exchange fails even if the connection was successful.
Currently, we fallback to RSockets only if the connected is rejected.
It turns out that WS server does not reject connection from our RSocket client. Instead, it rejects the messages.
As result, requestSignedCertFromFlipper fails, but it never triggers the change of protocol.
With this diff we start evaluating if we need to change our protocol for every socket error.
Reviewed By: lblasa
Differential Revision: D33890235
fbshipit-source-id: f79b5c6992f01f8a93e0793e180a5cbd4a105619
Summary:
Changelog: Initiate cert exchange when medium changes
Fixes the following bug:
1. Start an iOS app with WWW cert exchange
2. Get cert (and fake serial of a virtual device)
3. Close app
4. Restart Flipper
5. Switch app to FS_ACCESS cert exchange
6. Start app
Expected result:
App re-initializes cert exchange to get a new serial of a real device
Current result:
App tries to connect securely using the previous fake serial of a virtual device. Flipper does not recognize it and refuses the connection.
Reviewed By: lblasa
Differential Revision: D33163798
fbshipit-source-id: 67126a9b562f2cb7cfe6521a46abf38b2699fb2d
Summary:
This change makes WebSockets the default for Flipper on iOS.
Having said that, we are introducing some logic to deal with clients connecting to older Flipper Desktop versions.
The mobile client will first attempt to connect via WebSocket with the Desktop. This connection can either be secure or insecure. If that fails, it will attempt to connect via RSocket.
Connection failure logic:
The mobile client will attempt to connect up-to 3 times via a WebSocket. If it fails to connect, then the socket provider is switched to RSocket.
As before, the mobile client will attempt to connect up-to 3 times via a RSocket. If it fails to connect, then the socket provider is switched back to WebSocket.
Process repeats until a successful connection is established.
Some logs that can be seen from iOS:
2021-09-15 14:31:51.193503+0100 Sample[92026:92107440] [] nw_protocol_get_quic_image_block_invoke dlopen libquic failed
2021-09-15 14:31:51.878257+0100 Sample[92026:92107440] [connection] nw_socket_handle_socket_event [C1.1:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:52.553729+0100 Sample[92026:92107440] [connection] nw_socket_handle_socket_event [C1.2:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:52.899511+0100 Sample[92026:92107442] [connection] nw_connection_get_connected_socket [C1] Client called nw_connection_get_connected_socket on unconnected nw_connection
2021-09-15 14:31:52.899664+0100 Sample[92026:92107442] TCP Conn 0x600001d384d0 Failed : error 0:61 [61]
2021-09-15 14:31:57.120120+0100 Sample[92026:92107439] [connection] nw_socket_handle_socket_event [C2.1:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:57.141785+0100 Sample[92026:92107439] [connection] nw_socket_handle_socket_event [C2.2:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:57.151604+0100 Sample[92026:92107483] [connection] nw_connection_get_connected_socket [C2] Client called nw_connection_get_connected_socket on unconnected nw_connection
2021-09-15 14:31:57.154312+0100 Sample[92026:92107483] TCP Conn 0x600001d7c0b0 Failed : error 0:61 [61]
2021-09-15 14:31:59.206079+0100 Sample[92026:92107483] [connection] nw_socket_handle_socket_event [C3.1:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:59.236824+0100 Sample[92026:92107483] [connection] nw_socket_handle_socket_event [C3.2:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:59.251927+0100 Sample[92026:92107439] [connection] nw_connection_get_connected_socket [C3] Client called nw_connection_get_connected_socket on unconnected nw_connection
2021-09-15 14:31:59.255963+0100 Sample[92026:92107439] TCP Conn 0x600001d1c210 Failed : error 0:61 [61]
2021-09-15 14:32:01.291303+0100 Sample[92026:92107439] [connection] nw_socket_handle_socket_event [C4.1:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:32:01.312406+0100 Sample[92026:92107439] [connection] nw_socket_handle_socket_event [C4.2:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:32:01.323099+0100 Sample[92026:92107483] [connection] nw_connection_get_connected_socket [C4] Client called nw_connection_get_connected_socket on unconnected nw_connection
2021-09-15 14:32:01.326028+0100 Sample[92026:92107483] TCP Conn 0x600001d7c0b0 Failed : error 0:61 [61]
flipper: Failed to connect with the current socket provider
flipper: Use legacy socket provider
flipper: FlipperClient::onConnected
Reviewed By: passy
Differential Revision: D30900471
fbshipit-source-id: 7c242ad71306803b050d0174fc22696bb74fdba5
Summary:
This change fixes a bug with the handled flag during the certificate exchange process.
Explanation:
handled was passed by reference as &handled
Once the function goes out of scope then the reference, well, it just becomes invalid (undefined behaviour)
In some cases, it appears as 'handled' because the reference is invalid and it happens to be 'true'.
Changelog: Fixed an issue where clients would randomly not connect to Flipper. Please update FlipperKit to 0.110.0 to apply the fix: https://fbflipper.com/docs/getting-started/react-native#using-the-latest-flipper-sdk
Reviewed By: mweststrate
Differential Revision: D31017592
fbshipit-source-id: c087a769fa23de1acfd3c198b4db4d6ccdb2be90
Summary:
Abstract the socket creation from FlipperConnectionManagerImpl. Instead, use FlipperSocketProvider.
There's a default provider which will always return RSocket sockets. This provider can be changed and thus can return other implementations.
Reviewed By: fabiomassimo
Differential Revision: D30396322
fbshipit-source-id: 0583865376809260b0240e5bd653d73f2fa514b1
Summary:
It's just bad that we give a naked pointer of the connection manager to other instances. If the connection manager gets deallocated, the instances keeping a pointer to it are doomed to crash.
This change creates a wrapper on top of the pointer that can be freely shared. On deallocation, the shared wrapper gets invalidated.
Reviewed By: timur-valiev
Differential Revision: D30398466
fbshipit-source-id: 8f228e7fbaebc0ea28921409de071b58bbb69f1e
Summary:
These changes abstract RSocket from FlipperConnectionManagerImpl.
This is achieved by defining FlipperSocket.
FlipperConnectionManagerImpl uses instances of FlipperSocket and RSocket is now contained within FlipperRSocket which implements FlipperSocket.
On the changes to follow, FlipperConnectionManagerImpl will no longer reference FlipperRSocket directly thus fully abstracting the socket implementation in use.
For reviewers:
- All of the RSocket code now lives in FlipperRSocket.
- Main changes are in FlipperConnectionManagerImpl. Lambdas are used to deal with events and message handling.
- There's some very minimal serialisation additions for payloads.
Reviewed By: jknoxville
Differential Revision: D30341076
fbshipit-source-id: 54bb4878967378490710c05f729cdd7f4cf08bb8
Summary:
RSocket plays nicely with Folly and OpenSSL.
Flipper WebSocket-client uses SocketRocket which instead relies on Apple's NSInputStream and NSOutputStream types.
SSL options can be set to secure the communication in both.
Unfortunately, Apple APIs are a bit limited on the supported cryptographic formats it can accept as arguments.
SSL options require the client certificate to be set in PKCS #12 format, contrary to the existing PEM format used by RSocket.
This change adds a method to the ConnectionContext which converts and saves the client certificate in PKCS #12 format.
The method is always expected to succeed as it will only be called once a valid client certificate is available. An unlikely failure will raise an exception.
Reviewed By: fabiomassimo
Differential Revision: D30074334
fbshipit-source-id: 91a475d080569cc339b649c7302b1f28793c7de7
Summary:
This diff adds upload and download logic for certs. It makes changes on both Flipper Client and Desktop side. With this we enable cert exchange through WWW.
Next Diffs:
1) Add Flipper state in cert provider for more debug data
2) Tests
Reviewed By: jknoxville
Differential Revision: D23092706
fbshipit-source-id: e576253606b64b62848b70203db7e09a3bd77fd9
Summary:
This diff adds a toggle setting in wilde which will enable certificate exchange through www.
Right now it just sends the information about which medium to be used for cert exchange to Flipper JS and its client side. But its implementation is not done yet.
### Flow for Wilde
Whenever user changes the setting(or when user logs out) we set the state of exchange medium and accordingly set/reset authtoken. Note at no given point we remove already existing certificates.
### Context for OSS
With this diff we introduce another way to do certificate exchange. Before this diff, we did certificate exchange by accessing the file system of app. But it turns out it's not possible to do that in applications signed by enterprise certs. Thus with this diff one can write their FlipperKitCertificateProvider and fetch the certificate from WWW.
Reviewed By: jknoxville
Differential Revision: D22896320
fbshipit-source-id: 55aef7028a62e71ba9c02f9f79acaab41d09c0c6
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
Summary:
Context: https://fb.workplace.com/groups/flippersupport/permalink/803808233433170/
A similar change was done earlier with the secure connection route.
This does the same for the insecure route.
Reviewed By: mweststrate
Differential Revision: D21227196
fbshipit-source-id: 844cb674b5b16033977f451bbc3d8bbc69732273
Summary:
Originally, Flipper would use exceptions for the control flow of determining if there's no desktop client available. It would catch an exception and then kick off another attempt. This is cool, but if you are debugging an application where Flipper is enabled, and you set a breakpoint to detect all thrown exceptions, then you'll see an exception thrown inside Flipper logic every couple of seconds. That makes it hard to debug what you're trying to do.
There's a workaround where you can simply open Flipper, but that's a bit annoying.
This diff changes the logic in Flipper to not use exceptions for the part of connection where it's doing the initial connect. We could apply this to `doCertificateExchange` as well, but that's actually an error there so I don't see the need to do it there as well.
Reviewed By: jknoxville
Differential Revision: D21016737
fbshipit-source-id: cbdfe2794b4d38a9e3b8304ebee845655bb26ae5
Summary:
The problem is that whenever an app is shutdown, and then reopened, the flipper dir gets reset when getting the CSR for connecting to flipper.
This causes the first connection attempt to fail always, and it goes through the whole cert exchange, taking longer than necessary.
Fixes it by loading the csr from disk if it's not loaded yet, without blowing away the whole certs state.
A side effect of this would be that as long as some file exists where the csr lives, flipper state would never get reset, so it wouldn't be able to fix itself automatically anymore. To keep that working, I've made `resetFlipperDir()` public and am calling it explicitly when starting certificate exchange. This should ensure that we still reset when we need to, but not unnecessarily.
The reason it went wrong is that getCSR used to be called only at cert exchange, when resetting and generating a new one was always desirable. However, when we shipped the fix for changeable android serials, it started to be used as a normal getter.
Reviewed By: timur-valiev
Differential Revision: D18834806
fbshipit-source-id: 56ca7e03e1aa9011f836bc9c021cf3048f7dc1e4
Summary:
There was an interesting problem in Flipper which I encountered. On closing our app we would deadlock. This was happening when joining the Flipper thread.
Upon investigation I found that it happened only when Flipper was not connected to the app. And digging in I found that it's because we are continually trying to connect the `FlipperClient` to the Flipper app when there is no connection. This works great, but it continues even if you have asked Flipper to terminate. So this meant that the `EventBase` never gets chance to shutdown, as it keeps getting new work to do.
The fix is to add a flag to `FlipperConnectionManagerImpl` to say if the conection is started or not. Then, if it's not started and we get a request to retry connection, then we will fail it.
Reviewed By: jknoxville
Differential Revision: D18780622
fbshipit-source-id: cce82cbb5c54e6d92ea16644c8a247bd2578ae26
Summary:
- Add CSR and CSR destination to request body
- (unrelated) Rearrange the list of include files
Reviewed By: jknoxville
Differential Revision: D17346429
fbshipit-source-id: ce775998884b73e82a931f4dd8986c659a892b51
Summary:
Don't fatal when trying to send a message to flipper that is too large.
Instead just continue to throw an exception, so the plugin will still be aware of the failure.
Reviewed By: cpojer, danielbuechele
Differential Revision: D17180334
fbshipit-source-id: 49c341a7cee579158be3878639083c6d19b3eaa3
Summary:
`/*` is the standard throughout open source code. For example, Firefox uses single /*: https://hg.mozilla.org/mozilla-central/file/21d22b2f541258d3d1cf96c7ba5ad73e96e616b5/gfx/ipc/CompositorWidgetVsyncObserver.cpp#l3
In addition, Rust considers `/**` to be a doc comment (similar to Javadoc) and having such a comment at the beginning of the file causes `rustc` to barf.
Note that some JavaScript tooling requires `/**`. This is OK since JavaScript files were not covered by the linter in the first place, but it would be good to have that tooling fixed too.
Reviewed By: zertosh
Differential Revision: D15640366
fbshipit-source-id: b4ed4599071516364d6109720750d6a43304c089
Summary:
Fixes T40544233
Exceptions should be thrown by value, not as a pointer, or else catching by reference (the recommended practice) won't work.
Reviewed By: priteshrnandgaonkar
Differential Revision: D14243337
fbshipit-source-id: 28677d7f1b3c025b67fb945f30116f3b751a89a8
Summary: Allows plugin to check if something is supported by that app, for example an extension command or a new feature, before trying to use it.
Reviewed By: passy
Differential Revision: D14225957
fbshipit-source-id: 3c5a29a06b56fe5f1da772824232547447872344
Summary: This info is still available in the diagnostic screen.
Reviewed By: passy
Differential Revision: D14164791
fbshipit-source-id: 1fc91da0b7437d530bddee6cbb5ea8c2d11f7b80
Summary: I got tripped up again with a physical iOS device that had the wrong date and time. SSL certs won't be valid. Adds a reminder to the diagnostic screen for anyone running into this in future.
Reviewed By: passy
Differential Revision: D14132411
fbshipit-source-id: 623120ef5252eae3e2fe0f6f653074f586f17e0e
