Repository navigation
fix(driver): fail commands instead of hanging when the Dart VM connection closes - #937
theluckystrike wants to merge 1 commit into
Conversation
…tion closes
rpc-websockets 10.0.1 keeps a pending call until a reply or its timeout
arrives, and executeSocketCommand sets no timeout. When the Dart VM
socket closes during a command (app crash or relaunch), the call never
settles and the Appium command hangs until newCommandTimeout.
IsolateSocket now rejects pending calls on `close`. rpc-websockets
emits `close` only after it set `ready` to false, so calls made after
the drop are already refused with "socket not ready" and only calls from
the dead socket are in the queue. The connect wrapper turns a rejection
into an undefined result. executeElementCommand now throws a clear error
for an empty reply instead of a TypeError on `data.isError`. The wrapper
also logged `JSON.stringify(e)`, which is `{}` for an Error. It now logs
the Error itself, so the server log names the cause the new message
points to.
Fixes appium#733
I ran these in driver/ on this commit (main base e13a971), Node 22.23.2.
```
$ npm test
main lib + new tests tests 7 pass 5 fail 2
rejects a pending command when the Dart VM connection closes test timed out after 5000ms
fails an element command instead of hanging when the Dart VM ... test timed out after 5000ms
this commit tests 7 pass 7 fail 0
$ npm run lint
exit 0
$ npm run format:check
All matched files use the correct format.
```
End to end I ran the call the Appium router makes,
`FlutterDriver.executeCommand('execute', 'flutter:waitForAbsent', [finder])`,
against a fake Dart VM service. It drops the socket mid-command and
never replies, the way a crashing app does.
```
main
info Connection to ws://127.0.0.1:53066/ws closed
executeCommand('execute', 'flutter:waitForAbsent') -> STILL PENDING after 15000 ms (hang)
this commit
info Connection to ws://127.0.0.1:53104/ws closed
ERR! Error: The Dart VM connection was closed before it replied
executeCommand('execute', 'flutter:waitForAbsent') -> rejected: Cannot execute command
waitForAbsent, no response from the Dart VM. Check the server log for the connection error [4 ms]
```
I didn't run the device functional workflow (driver-function.yml) because it
needs an iOS simulator with a built Flutter app.
|
|
KazuCocoa
left a comment
There was a problem hiding this comment.
Rejecting pending RPCs on disconnect addresses the hang. I reproduced two error-handling gaps that should be covered alongside this change.
| for (const [id, {promise, timeout}] of Object.entries(queue)) { | ||
| clearTimeout(timeout); | ||
| delete queue[id]; | ||
| promise[1](new Error(`The Dart VM connection was closed before it replied`)); |
There was a problem hiding this comment.
[P1] Handle disconnect rejection during initial VM discovery
This also rejects the initial getVM call in connectSocket when no isolateId is supplied. The socket.call wrapper catches that rejection and returns undefined, after which the async onOpenListener reads vm.isolates outside its try/catch. I reproduced an unhandled TypeError (Cannot read properties of undefined, reading 'isolates') by terminating the connection while getVM was pending; the connection promise remained unsettled. Please handle this setup failure and settle the connection promise rather than allowing the event listener to reject unhandled, and add a test for a disconnect during getVM.
| this.log.debug(`>>> ${JSON.stringify(serializedCommand)}`); | ||
| const data = await (this.socket as IsolateSocket).executeSocketCommand(serializedCommand); | ||
| this.log.debug(`<<< ${JSON.stringify(data)} | previous command ${command}`); | ||
| if (!data) { |
There was a problem hiding this comment.
[P2] Propagate disconnect failures to commands that bypass executeElementCommand
This guard only covers commands routed through executeElementCommand. enterText, requestData, setFrameSync, and other handlers call executeSocketCommand directly, so the socket.call wrapper still swallows the newly rejected pending call and resolves with undefined. Using a local RPC server that terminated the connection during ext.flutter.driver, I reproduced execute('flutter:enterText', ['hello']) resolving successfully with undefined instead of rejecting. Please propagate the disconnect failure through those paths too, with a regression test for a direct command handler.
Fixes #733
The log in #733 ends with
Connection to ws://.../ws closedright after>>> waitForAbsent, and the session then sits idle until UiAutomator2's newCommandTimeout. That's the driver waiting on a reply that can't arrive.IsolateSocket.executeSocketCommandcallsext.flutter.driverwith no timeout. In rpc-websockets 10.0.1 a pending call settles only on a reply or on its own timeout. The clientclosehandler marks the socket not ready and schedules a reconnect, but it never touches the pending queue. Any Dart VM disconnect during a command (app crash, relaunch, reinstall) leaves the Appium command pending forever.Changes
driver/lib/sessions/isolate_socket.tsrejects every pending call when the socket emitsclose. rpc-websockets emitscloseafter it setreadyto false, so calls made after the drop are refused with "socket not ready" and the queue only holds calls from the dead socket.driver/lib/sessions/observatory.tsthrowsCannot execute command <name>, no response from the Dart VMwhen the reply is empty. Thesocket.callwrapper inconnectSocketcatches the rejection and returnsundefined, and without this guard the command fails with a TypeError ondata.isError.JSON.stringify(e), which prints{}for an Error. It now logs the Error itself, so the server log showsThe Dart VM connection was closed before it replied.driver/test/isolate-socket.test.mjsadds 2 tests against an rpc-websocketsServeracting as the Dart VM service. It drops the client whileext.flutter.driveris pending.What I ran in
driver/on Node 22.23.2End to end I ran the call the Appium router makes,
FlutterDriver.executeCommand('execute', 'flutter:waitForAbsent', [finder]), against the same fake Dart VM.The device workflow (driver-function.yml) needs a simulator with a Flutter app and wasn't run here. The reconnect behavior of rpc-websockets stays as it is, and a per-command timeout (#430) is a separate change.