Skip to content

[java] remove references to invalid errors#10209

Merged
diemol merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:error_codec
Feb 22, 2022
Merged

[java] remove references to invalid errors#10209
diemol merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:error_codec

Conversation

@titusfortner
Copy link
Copy Markdown
Member

I ran across a reference to an invalid error in something else I was working on and realized the ErrorCodec list is out of date with current w3c errors. I think these errors are getting used by the grid? I don't see references to them in the code base otherwise.

I'm not sure what is needed for backwards compatibility, or why the invalid ones aren't causing problems, and why the ShadowRootException not being on this list isn't an issue, and what tests we're missing for these one way or another.

This would all be much easier if we could remove JWP support like we'd originally planned.

This PR will run the Java tests and I'll see if there's anything important this breaks (hopefully). Otherwise let me know if I've gotten too delete happy.

@titusfortner titusfortner added B-grid Everything grid and server related C-java Java Bindings labels Dec 31, 2021
Copy link
Copy Markdown
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check the conflicts, @titusfortner?

@titusfortner titusfortner force-pushed the error_codec branch 2 times, most recently from a875592 to ea30ac0 Compare February 22, 2022 19:15
@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@diemol diemol merged commit 45bb38a into SeleniumHQ:trunk Feb 22, 2022
@surli
Copy link
Copy Markdown

surli commented May 9, 2022

Hi, we upgraded recently to Selenium 4.1.3 in a project which uses Selenium API to build its API, and I was using ElementNotVisibleException in this API whenever some checks were not met. AFAICS I just have to change my API to return ElementNotInteractableException so not a big deal to me, but it's a bit of a surprise that such an exception has been removed in a minor version while it wasn't even deprecated.
[Related commit: https://github.com/xwiki-contrib/application-changerequest/commit/5373b6a8e42205402f03017e7eeaf9e0529fc116 ]

@trevorgulick-home
Copy link
Copy Markdown

Likewise for ElementNotSelectableException. Could there be a more visible reference to this change created somewhere so that devs know to use ElementNotInteractableException instead of these classes going forward? Otherwise they will end up here to answer the question as to what happened to these exception classes.

@surli
Copy link
Copy Markdown

surli commented May 10, 2022

Could there be a more visible reference to this change created somewhere so that devs know to use ElementNotInteractableException instead of these classes going forward?

Took me a bit of time to spot this PR for the change, ideally I would have preferred seeing it directly in the changelog.

@diemol
Copy link
Copy Markdown
Member

diemol commented May 10, 2022

It is in the changelog https://github.com/SeleniumHQ/selenium/blob/trunk/java/CHANGELOG#L38

However, yes, this was a mistake to have released this in a patch release. Apologies and we will try to avoid this in the future.

@titusfortner
Copy link
Copy Markdown
Member Author

Well, we're not doing SemVer, so we're not making promises about what will happen in a patch release vs a minor release.

That being said, I *did say that it probably had unintended consequences I couldn't see right off. It would be nice if we had tests for this. @surli what was the context for the errors you saw? Was it via the Grid? I don't see where the Java code itself was actually using those errors.

@surli
Copy link
Copy Markdown

surli commented May 23, 2022

Well, we're not doing SemVer, so we're not making promises about what will happen in a patch release vs a minor release.

Indeed, now you could have maybe just marked it as deprecated first? I think I remember some API being deprecated before their removal in selenium, but maybe I'm wrong.

I don't see where the Java code itself was actually using those errors.

So maybe I haven't been clear, but it wasn't a bug in Selenium itself, but in my own project which build an API on top of it, see the commit where I fixed it: xwiki-contrib/application-changerequest@5373b6a

@titusfortner
Copy link
Copy Markdown
Member Author

Yes, you're completely correct, these should have been marked as deprecated before getting removed.
Sorry for the extra hassle here.

labkey-tchad added a commit to LabKey/testAutomation that referenced this pull request May 31, 2022
Remove references to removed selenium exceptions
SeleniumHQ/selenium#10209
elgatov pushed a commit to elgatov/selenium that referenced this pull request Jun 27, 2022
labkey-tchad added a commit to LabKey/testAutomation that referenced this pull request Jul 29, 2022
* Update to Selenium 4.3
Remove references to removed selenium exceptions
SeleniumHQ/selenium#10209
@titusfortner titusfortner deleted the error_codec branch December 4, 2022 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-grid Everything grid and server related C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants