appium / WebDriverAgent

A WebDriver server for iOS and tvOS
Other
1.18k stars 368 forks source link

chore: Handle app startup errors as session creation exceptions #855

Closed mykola-mokhnach closed 6 months ago

mykola-mokhnach commented 6 months ago

.

mykola-mokhnach commented 6 months ago

@KazuCocoa @Dan-Maor Could you please help to test this scenario on a real device?

KazuCocoa commented 6 months ago

i tried to get this situation, but the [self launch]; succeeded without issues, so almost immediately self.fb_didStartWithoutBlockingAlert = @YES; was set (and succeeded).

My tested case was to launch a safari while showing a location permission alert.

If that is the case then calling [XCUIApplication launch] blocks forever.

Do you have any idea what app/alert could cause this case? @mykola-mokhnach

KazuCocoa commented 6 months ago

when i forcefully removed self.fb_didStartWithoutBlockingAlert = @YES; and kept running the alert check, actually the The application '%@' cannot be launched because it is blocked by an unexpected system alert: %@ was printed. So maybe when the [self launch]; was actually blocked by alert, this should print the error message properly

mykola-mokhnach commented 6 months ago

i tried to get this situation, but the [self launch]; succeeded without issues, so almost immediately self.fb_didStartWithoutBlockingAlert = @YES; was set (and succeeded).

My tested case was to launch a safari while showing a location permission alert.

If that is the case then calling [XCUIApplication launch] blocks forever.

Do you have any idea what app/alert could cause this case? @mykola-mokhnach

Like I described you could try it with any app signed with enterprise signature, whose enterprise profile has not been accepted on the device yet.

Another possible test case: make springboard to show any alert, for example ios update one and try to install an app without accepting the alert manually. WDA must be already running on the device while doing this

KazuCocoa commented 6 months ago

let me find possible alerts. Permission dialogs by springboard did not block [self launch]; on my env, so self.fb_didStartWithoutBlockingAlert = @YES; was called almost immediately. Actually free account's code sign could be the candidate as a handy method.

Dan-Maor commented 6 months ago

I've ran some tests this morning by triggering an untrusted enterprise certificate popup and it didn't block session creation for other applications (tested on a real device running iOS 17.4), so I think this functionality may not be necessary, at least for real devices. Given that it generates additional payload via accessibility operations, I think this mechanism - if the issue can be reproduced on simulators - should be limited for simulators only. What do you think?

KazuCocoa commented 6 months ago

it is reasonable for me, or I wonder if this Pr can be just one time alert existence check in https://github.com/appium/WebDriverAgent/pull/855/files#diff-468cd0130bf820fda63ed058220b6b951c0a59ad6e6ad009b1d67daa8b037ca7R97-R99 and add the alert existence in the error response. Then users can notice something alert could remain on the device since such trusted popup could remain on the device, and next time should be "cancel"ed

mykola-mokhnach commented 6 months ago

I've ran some tests this morning by triggering an untrusted enterprise certificate popup and it didn't block session creation for other applications (tested on a real device running iOS 17.4), so I think this functionality may not be necessary, at least for real devices. Given that it generates additional payload via accessibility operations, I think this mechanism - if the issue can be reproduced on simulators - should be limited for simulators only. What do you think?

Thanks for the feedback @Dan-Maor I will change the PR to reflect this.

mykola-mokhnach commented 6 months ago

I have removed the alert presence check. Looks like it was probably a bug in earlier iOS versions. The only change now is that app launch errors are mapped to a webdriver session creation exception

github-actions[bot] commented 6 months ago

:tada: This PR is included in version 7.0.6 :tada:

The release is available on:

Your semantic-release bot :package::rocket: