fix(webdriverio): don't fail if getContext is not supported by christian-bromann · Pull Request #13950 · webdriverio/webdriverio

christian-bromann · 2024-12-06T00:43:25Z

Proposed changes

We've had folks reporting that they get an error like this:

This patch mitigates this problem by just catching this error.

Types of changes

Polish (an improvement for an existing feature)
Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update (improvements to the project's docs)
Specification changes (updates to WebDriver command specifications)
Internal updates (everything related to internal scripts, governance documentation and CI files)

Checklist

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have added the necessary documentation (if appropriate)
I have added proper type definitions for new commands (if appropriate)

Backport Request

//: # (The current main branch is the development branch for WebdriverIO v9. If your change should be released to the current major version of WebdriverIO (v8), please raise another PR with the same changes against the v8 branch.)

This change is solely for v9 and doesn't need to be back-ported
Back-ported PR at #XXXXX

Further comments

n/a

Reviewers: @webdriverio/project-committers

pkg-pr-new · 2024-12-06T00:46:24Z

Open in Stackblitz

eslint-plugin-wdio

npm i https://pkg.pr.new/webdriverio/webdriverio/eslint-plugin-wdio@13950

@wdio/appium-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/appium-service@13950

@wdio/browser-runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/browser-runner@13950

@wdio/allure-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/allure-reporter@13950

@wdio/browserstack-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/browserstack-service@13950

@wdio/cli

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/cli@13950

@wdio/concise-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/concise-reporter@13950

@wdio/config

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/config@13950

@wdio/cucumber-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/cucumber-framework@13950

@wdio/firefox-profile-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/firefox-profile-service@13950

@wdio/dot-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/dot-reporter@13950

@wdio/globals

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/globals@13950

@wdio/jasmine-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/jasmine-framework@13950

@wdio/json-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/json-reporter@13950

@wdio/junit-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/junit-reporter@13950

@wdio/lighthouse-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/lighthouse-service@13950

@wdio/local-runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/local-runner@13950

@wdio/logger

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/logger@13950

@wdio/mocha-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/mocha-framework@13950

@wdio/protocols

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/protocols@13950

@wdio/repl

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/repl@13950

@wdio/reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/reporter@13950

@wdio/runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/runner@13950

@wdio/sauce-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/sauce-service@13950

@wdio/shared-store-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/shared-store-service@13950

@wdio/smoke-test-cjs-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-cjs-service@13950

@wdio/smoke-test-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-reporter@13950

@wdio/smoke-test-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-service@13950

@wdio/spec-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/spec-reporter@13950

@wdio/static-server-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/static-server-service@13950

@wdio/sumologic-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/sumologic-reporter@13950

@wdio/testingbot-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/testingbot-service@13950

@wdio/types

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/types@13950

@wdio/utils

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/utils@13950

@wdio/webdriver-mock-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/webdriver-mock-service@13950

webdriver

npm i https://pkg.pr.new/webdriverio/webdriverio/webdriver@13950

webdriverio

npm i https://pkg.pr.new/webdriverio/webdriverio@13950

commit: 619e47d

wswebcreation

Hi @christian-bromann , see my comment

wswebcreation · 2024-12-06T05:17:15Z

packages/webdriverio/src/context.ts

-                context : typeof context === 'object' ?
-                    context.id :
-                    undefined
+            const context = await this.#browser.getContext().catch((err) => {


@christian-bromann

Do you also have the context of when this happens, this is not something that should occur when using Appium, unless they are for example

using wrong caps/WDIO gets back non W3C ones

the driver is old as ...

I think generally we can't make the assumption that just because a driver is considered as "mobile" they have the getContext command implemented. A driver can always fail to send the correct capabilities causing WebdriverIO to consider a session as mobile when it isn't. This patch will continue to log and elevate the problem but won't cause the whole session to fail.

What do you think?

So my two-cents:
We then might need to:

Fix the isMobile check to make it more robust

Do this "fallback" for all commands

I also believe we can't "patch" the vendors (because this doesn't seem to be a specific Appium issue, unless a very old driver is used) if they don't return "valid" W3C responses.

Fix the isMobile check to make it more robust

We should totally strive for that but unfortunately can't rely on vendors implementing the specifications correctly.

Do this "fallback" for all commands

What do you mean by "all commands"? If some one uses an unsupported command in their test, it should fail. However with these session manager (e.g. context, shadow root manager or the polyfill manager) we are enhancing WebdriverIO in the background and these things shouldn't cause the testrun to fail. Hence this patch which makes the context manager do nothing if getContext is not implemented.

if they don't return "valid" W3C responses.

I think this is not the issue. The issue is that the command is not implemented by the vendor or WebDriver server in which case we should disable the context manager.

What do you mean by "all commands"? If some one uses an unsupported command

I mean for mobile commands specific commands. But I agree with what you say

I think this is not the issue. The issue is that the command is not implemented by the vendor or WebDriver server in which case we should disable the context manager.

I believe you just fixed this "not implemented by WebDriver" with #13949, so I wonder if this "fix" is still needed. I would then still let it fail and throw a different error message (maybe throw the capabilities that are used) to determine which drivers are not supporting this. Appium drivers (especially the most used ones) should support the "browser" commands AND the "native" commands. getContext is one of the oldest API's
Regarding the vendors, from what I now see LT is doing it differently, they default to JSONWP

and the W3C caps are not "Appium" proof

I'll put checking the vendors on my todo list so we can properly check if it's mobile based on the correct caps

I believe you just fixed this "not implemented by WebDriver" with #13949, so I wonder if this "fix" is still needed.

I put up this patch because I am afraid we run into more issues where our mobile capability detection fails and causes a failure with the whole test run. I think this patch makes sense no matter what because we just disable the context manager in case the command fails.

I believe you just fixed this "not implemented by WebDriver" with #13949, so I wonder if this "fix" is still needed.

I put up this patch because I am afraid we run into more issues where our mobile capability detection fails and causes a failure with the whole test run. I think this patch makes sense no matter what because we just disable the context manager in case the command fails.

Then can you add extra logs for debugging purposes so we can update the logic that is behind this when we get issues?

What would you suggest aside from the

log.warn(`Error getting context: ${err}`)

?

Maybe also throw the requested capabilities so we can determine if the mobile checks have made an error

wswebcreation

Thanks!

christian-bromann added the PR: Polish 💅 PRs that contain improvements on existing features label Dec 6, 2024

wswebcreation reviewed Dec 6, 2024

View reviewed changes

christian-bromann added 2 commits December 10, 2024 16:57

fix(webdriverio): don't fail if getContext is not supported

39a1102

tweak

9eeec29

christian-bromann force-pushed the cb/getContext-fallback branch from 06e30b9 to 9eeec29 Compare December 10, 2024 19:58

log capabilities

95ca64f

wswebcreation approved these changes Dec 11, 2024

View reviewed changes

Merge branch 'main' into cb/getContext-fallback

619e47d

christian-bromann merged commit 7e90730 into main Dec 11, 2024

christian-bromann deleted the cb/getContext-fallback branch December 11, 2024 19:51

Uh oh!

Conversation

christian-bromann commented Dec 6, 2024

Proposed changes

Types of changes

Checklist

Backport Request

Further comments

Reviewers: @webdriverio/project-committers

Uh oh!

pkg-pr-new bot commented Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wswebcreation left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wswebcreation Dec 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christian-bromann Dec 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wswebcreation left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pkg-pr-new bot commented Dec 6, 2024 •

edited

Loading

wswebcreation Dec 9, 2024 •

edited

Loading

christian-bromann Dec 10, 2024 •

edited

Loading