From 3d8ccf93352fdcaa2906be3510be6e347adb5cc5 Mon Sep 17 00:00:00 2001 From: timo <44401485+Timo972@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:18:09 +0200 Subject: [PATCH] fix(mobilerun): keep a failed state snapshot from answering 404 GET /mobilerun/state let a failed application snapshot fall through to the global exception mapping, which answers FBStaleElementException with 404. That is also what a stock WebDriverAgent answers for the missing route, so mobilerun-ios latched "no /mobilerun/state on this runner" and served every later /state from the /source dump: 5+ s (12+ s on SpringBoard), and with the display name or nothing as packageName, since the source root carries no bundle ID. The snapshot fails right after a Home press, when the app resolved at the start of the request is the one leaving the foreground. Retry once with the active app resolved anew, and if that fails too answer 400 (invalid element state): neither the route-missing 404 nor a wedged-runner 5xx. Refs DRO-3213 Co-Authored-By: Claude Opus 5.5 (1M context) --- WebDriverAgent.xcodeproj/project.pbxproj | 4 + .../Commands/FBMobilerunA11yCommands.h | 17 ++++ .../Commands/FBMobilerunA11yCommands.m | 37 +++++++- .../UnitTests/FBMobilerunA11yCommandsTests.m | 91 +++++++++++++++++++ 4 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 WebDriverAgentTests/UnitTests/FBMobilerunA11yCommandsTests.m diff --git a/WebDriverAgent.xcodeproj/project.pbxproj b/WebDriverAgent.xcodeproj/project.pbxproj index 506a63132..1fa113b44 100644 --- a/WebDriverAgent.xcodeproj/project.pbxproj +++ b/WebDriverAgent.xcodeproj/project.pbxproj @@ -1333,6 +1333,7 @@ FBCAFE000000000000006042 /* FBExtAudioPipeline.m in Sources */ = {isa = PBXBuildFile; fileRef = FBCAFE000000000000006041 /* FBExtAudioPipeline.m */; }; FBCAFE000000000000006051 /* FBAudioStreamTests.m in Sources */ = {isa = PBXBuildFile; fileRef = FBCAFE000000000000006050 /* FBAudioStreamTests.m */; }; FBCAFE000000000000008051 /* FBBroadcastLabelsTests.m in Sources */ = {isa = PBXBuildFile; fileRef = FBCAFE000000000000008050 /* FBBroadcastLabelsTests.m */; }; + FBCAFE000000000000008053 /* FBMobilerunA11yCommandsTests.m in Sources */ = {isa = PBXBuildFile; fileRef = FBCAFE000000000000008052 /* FBMobilerunA11yCommandsTests.m */; }; FBCAFE000000000000007002 /* FBSocks5TunnelProtocol.h in Headers */ = {isa = PBXBuildFile; fileRef = FBCAFE000000000000007000 /* FBSocks5TunnelProtocol.h */; }; FBCAFE000000000000007003 /* FBSocks5TunnelProtocol.h in Headers */ = {isa = PBXBuildFile; fileRef = FBCAFE000000000000007000 /* FBSocks5TunnelProtocol.h */; }; FBCAFE000000000000007004 /* FBSocks5TunnelProtocol.m in Sources */ = {isa = PBXBuildFile; fileRef = FBCAFE000000000000007001 /* FBSocks5TunnelProtocol.m */; }; @@ -2111,6 +2112,7 @@ FBCAFE000000000000006041 /* FBExtAudioPipeline.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FBExtAudioPipeline.m; sourceTree = ""; }; FBCAFE000000000000006050 /* FBAudioStreamTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FBAudioStreamTests.m; sourceTree = ""; }; FBCAFE000000000000008050 /* FBBroadcastLabelsTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FBBroadcastLabelsTests.m; sourceTree = ""; }; + FBCAFE000000000000008052 /* FBMobilerunA11yCommandsTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FBMobilerunA11yCommandsTests.m; sourceTree = ""; }; FBCAFE000000000000007000 /* FBSocks5TunnelProtocol.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FBSocks5TunnelProtocol.h; sourceTree = ""; }; FBCAFE000000000000007001 /* FBSocks5TunnelProtocol.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FBSocks5TunnelProtocol.m; sourceTree = ""; }; FBCAFE000000000000007010 /* FBSocks5URI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FBSocks5URI.h; sourceTree = ""; }; @@ -2958,6 +2960,7 @@ FBCAFE000000000000004001 /* FBVideoStreamSessionTests.m */, FBCAFE000000000000006050 /* FBAudioStreamTests.m */, FBCAFE000000000000008050 /* FBBroadcastLabelsTests.m */, + FBCAFE000000000000008052 /* FBMobilerunA11yCommandsTests.m */, FBCAFE000000000000002001 /* FBImageUtilsTests.m */, ); path = UnitTests; @@ -5276,6 +5279,7 @@ FBCAFE000000000000004002 /* FBVideoStreamSessionTests.m in Sources */, FBCAFE000000000000006051 /* FBAudioStreamTests.m in Sources */, FBCAFE000000000000008051 /* FBBroadcastLabelsTests.m in Sources */, + FBCAFE000000000000008053 /* FBMobilerunA11yCommandsTests.m in Sources */, FBCAFE000000000000002002 /* FBImageUtilsTests.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/WebDriverAgentLib/Commands/FBMobilerunA11yCommands.h b/WebDriverAgentLib/Commands/FBMobilerunA11yCommands.h index b6d1c0e71..62210e19c 100644 --- a/WebDriverAgentLib/Commands/FBMobilerunA11yCommands.h +++ b/WebDriverAgentLib/Commands/FBMobilerunA11yCommands.h @@ -12,8 +12,25 @@ NS_ASSUME_NONNULL_BEGIN +@class FBCommandStatus; + @interface FBMobilerunA11yCommands : NSObject +/** + Builds the GET /mobilerun/state value by running 'attempt' - one snapshot walk of the active + application - and retrying it once when the snapshot fails (FBStaleElementException). + + Exposed for unit testing; the route handler passes a block that resolves the active application + anew on every call. + + @param attempt returns the state dictionary; may throw + @param failure set when both attempts failed to snapshot the application + @return the state dictionary, or nil with 'failure' set. Exceptions other than a failed snapshot + are rethrown unchanged. + */ ++ (nullable NSDictionary *)stateWithSnapshotAttempt:(NSDictionary *(NS_NOESCAPE ^)(void))attempt + failure:(FBCommandStatus *_Nullable *_Nullable)failure; + @end NS_ASSUME_NONNULL_END diff --git a/WebDriverAgentLib/Commands/FBMobilerunA11yCommands.m b/WebDriverAgentLib/Commands/FBMobilerunA11yCommands.m index adc19a3d9..ea9d98d88 100644 --- a/WebDriverAgentLib/Commands/FBMobilerunA11yCommands.m +++ b/WebDriverAgentLib/Commands/FBMobilerunA11yCommands.m @@ -9,6 +9,7 @@ #import "FBMobilerunA11yCommands.h" #import "FBCommandStatus.h" +#import "FBExceptions.h" #import "FBResponsePayload.h" #import "FBRoute.h" #import "FBRouteRequest.h" @@ -52,15 +53,47 @@ + (NSArray *)routes #pragma mark - Commands ++ (NSDictionary *)stateWithSnapshotAttempt:(NSDictionary *(NS_NOESCAPE ^)(void))attempt + failure:(FBCommandStatus **)failure +{ + NSException *snapshotFailure = nil; + for (NSUInteger attemptIndex = 0; attemptIndex < 2; attemptIndex++) { + @try { + return attempt(); + } @catch (NSException *exception) { + if (![exception.name isEqualToString:FBStaleElementException]) { + @throw; + } + snapshotFailure = exception; + } + } + // Left to the global exception mapping, a failed snapshot answers 404 (stale element reference), + // which is also what a stock WebDriverAgent answers for this missing route - so clients take it + // as "no /mobilerun/state here" and switch to the far slower /source for good. A 5xx would read + // as a wedged runner instead. The app was just not in a state that can be snapshotted. + if (failure) { + *failure = [FBCommandStatus invalidElementStateErrorWithMessage:snapshotFailure.reason + traceback:nil]; + } + return nil; +} + + (id)handleGetState:(FBRouteRequest *)request { - XCUIApplication *app = request.session.activeApplication ?: XCUIApplication.fb_activeApplication; CGFloat scale; if (!FBMobilerunScaleFromRequest(request, &scale)) { return FBResponseWithStatus([FBCommandStatus invalidArgumentErrorWithMessage:@"'scale' must be a positive number" traceback:nil]); } - return FBResponseWithObject([app fb_mobilerunA11yStateWithScale:scale]); + FBCommandStatus *failure = nil; + NSDictionary *state = [self stateWithSnapshotAttempt:^NSDictionary *{ + // Resolved on every attempt: right after a Home press the app picked at the start of the + // request can be the one leaving the foreground, whose snapshot then fails. The retry resolves + // the app that took over (SpringBoard). + XCUIApplication *app = request.session.activeApplication ?: XCUIApplication.fb_activeApplication; + return [app fb_mobilerunA11yStateWithScale:scale]; + } failure:&failure]; + return nil == state ? FBResponseWithStatus(failure) : FBResponseWithObject(state); } @end diff --git a/WebDriverAgentTests/UnitTests/FBMobilerunA11yCommandsTests.m b/WebDriverAgentTests/UnitTests/FBMobilerunA11yCommandsTests.m new file mode 100644 index 000000000..d674a9e9f --- /dev/null +++ b/WebDriverAgentTests/UnitTests/FBMobilerunA11yCommandsTests.m @@ -0,0 +1,91 @@ +/** + * Copyright (c) 2026-present, Droidrun. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import + +#import "FBCommandStatus.h" +#import "FBExceptions.h" +#import "FBHTTPStatusCodes.h" +#import "FBMobilerunA11yCommands.h" + +@interface FBMobilerunA11yCommandsTests : XCTestCase +@end + +@implementation FBMobilerunA11yCommandsTests + +static NSException *FBStaleSnapshotException(void) +{ + return [NSException exceptionWithName:FBStaleElementException reason:@"snapshot failed" userInfo:nil]; +} + +- (void)testStateIsReturnedWhenTheFirstSnapshotSucceeds +{ + __block NSUInteger attempts = 0; + FBCommandStatus *failure = nil; + NSDictionary *state = [FBMobilerunA11yCommands stateWithSnapshotAttempt:^NSDictionary *{ + attempts++; + return @{@"phone_state": @{}}; + } failure:&failure]; + + XCTAssertEqualObjects(state, @{@"phone_state": @{}}); + XCTAssertNil(failure); + XCTAssertEqual(attempts, 1u); +} + +- (void)testFailedSnapshotIsRetriedOnce +{ + // After a Home press the app resolved at the start of the request can be the one that is + // leaving the foreground; its snapshot fails, and a second attempt resolves SpringBoard. + __block NSUInteger attempts = 0; + FBCommandStatus *failure = nil; + NSDictionary *state = [FBMobilerunA11yCommands stateWithSnapshotAttempt:^NSDictionary *{ + if (0 == attempts++) { + @throw FBStaleSnapshotException(); + } + return @{@"phone_state": @{@"packageName": @"com.apple.springboard"}}; + } failure:&failure]; + + XCTAssertEqualObjects(state[@"phone_state"][@"packageName"], @"com.apple.springboard"); + XCTAssertNil(failure); + XCTAssertEqual(attempts, 2u); +} + +- (void)testPersistentSnapshotFailureIsNotReportedAsNotFound +{ + // A 404 is what a stock WebDriverAgent answers for the missing route, and clients take it as + // "this runner has no /mobilerun/state" for good. A failed snapshot must say something else, + // and must not be a 5xx either: that reads as a wedged runner and triggers a relaunch. + __block NSUInteger attempts = 0; + FBCommandStatus *failure = nil; + NSDictionary *state = [FBMobilerunA11yCommands stateWithSnapshotAttempt:^NSDictionary *{ + attempts++; + @throw FBStaleSnapshotException(); + } failure:&failure]; + + XCTAssertNil(state); + XCTAssertEqual(attempts, 2u); + XCTAssertNotNil(failure); + XCTAssertNotEqual(failure.statusCode, kHTTPStatusCodeNotFound); + XCTAssertGreaterThanOrEqual(failure.statusCode, 400); + XCTAssertLessThan(failure.statusCode, 500); + XCTAssertTrue([failure.message containsString:@"snapshot failed"]); +} + +- (void)testOtherExceptionsAreNotSwallowed +{ + // Anything but a failed snapshot keeps the global exception mapping (a deadlocked app, say). + __block NSUInteger attempts = 0; + FBCommandStatus *failure = nil; + XCTAssertThrowsSpecificNamed(([FBMobilerunA11yCommands stateWithSnapshotAttempt:^NSDictionary *{ + attempts++; + @throw [NSException exceptionWithName:FBApplicationDeadlockDetectedException reason:@"" userInfo:nil]; + } failure:&failure]), NSException, FBApplicationDeadlockDetectedException); + XCTAssertEqual(attempts, 1u); +} + +@end