[🐛 Bug]: Cookies management is not consistent

[nvborisenko]

What happened?

In C#:

driver.FindElement(By.CssSelector("abc")); // throws NoSuchElement

driver.Manage().Cookies.GetCookieNamed("abc"); // null if not found

But according spec it should throw an exception: https://www.w3.org/TR/webdriver2/#get-named-cookie

How can we reproduce the issue?

It is about API consistency

Relevant log output

No output

Operating System

any

Selenium version

any

What are the browser(s) and version(s) where you see this issue?

any

What are the browser driver(s) and version(s) where you see this issue?

any

Are you using Selenium Grid?

No response

[github-actions]

@nvborisenko, thank you for creating this issue. We will troubleshoot it as soon as we can.

---

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

[nvborisenko]

From C# perspective it would be nice to:

• leave it as is (and mark it as deprecated)
• new method GetCookie(string name) which throws if not found
• new method bool TryGetCookie(string name, out Cookie cookie) which is known pattern

[p0deje]

I would consider this to be a common API change that needs to follow the deprecation process:

1. Deprecate the current method if it returns null with a message that it will start throwing an error in the upcoming releases.
2. Wait for 2-3 releases.
3. Replace implementation.

I don't think Try* is necessary, but I've never used Selenium directly and always used a wrapper (Watir, Capybara). These libraries can easily handle the exception internally.

[nvborisenko]

Current API is bad, it hides real reason under hood. Let's compare... I want to:

• Delete file by its path
• Get an element from dictionary by key
• Connect to remote network point by address
• Remove an element from an array by index = 100, where the array has only 50 elements

In all cases it throws/raises exception (in all programming languages?), like "Dear user, you are doing something wrong". Selenium should not be silent. #14545 shows why. When user deletes cookie via .DeleteCookieNamed("abc") and this method returns null: does it mean the cookie is deleted successfully or the cookie doesn't exists - no clues. It should be predictable.

[RenderMichael]

Taking a look at how users actually use this method in the wild: https://grep.app/search?q=GetCookieNamed%28

It looks like more often than not, people are just making an assertion that a cookie does not exist. In other words, following the WebDriver spec would require making users catch an exception (or more specifically, assert that an exception is thrown).

If we don’t want a Try*, method, can we at least have a ContainsCookieNamed method? that would bridge the gap for (what seems to me as) the majority use case.

[nvborisenko]

I don't think we are talking about Try* here. You are still able to do: .AllCookies.Where(c => c.Name.StartsWith("a")). It can be easily mitigated.

Returning null silently is a point.

[pujagani]

I think we should switch to using https://www.w3.org/TR/webdriver2/#get-named-cookie but maintain the backward compatibility. Throwing an exception if the cookie is not there would be a breaking change and might require users to update their assertions and tests. In my observation, end-users typically resist updating if there is a huge change and this might be that huge change.

Selenium usage: https://plausible.io/manager.selenium.dev
I don't think analyzing just Github repositories is the best way to gauge the usage of this API. I am sure there are way more private repos/corporate company repos that use it differently and there is no way for us to find out.

[diemol]

I agree that returning null is not ideal. But we cannot change 12 years of behavior in a single release.

I like the approach of deprecating the method and creating a new one using the new endpoint.

Like that, we will get feedback when people see the deprecation. Similarly to what is happening with getAttribute.

[bonigarcia]

Breaking changes like this can be expected in a new major release, so I would make this change (and others, potentially) in Selenium 5. This new major release can be a good opportunity to revamp/modernize the Selenium API across all the bindings. I know there is no official date for Selenium 5, and it is difficult to forecast, but this may happen in 2025, right?

[pujagani]

Now that I think of it, it might be a good idea to align the breaking change with a major release and revamp the API.
Meanwhile, we can switch the API to use https://www.w3.org/TR/webdriver2/#get-named-cookie internally.

[navin772]

I can take this up for Python bindings if its needed!

[nvborisenko]

Interesting important fact, which influences to the decision.

According specification:

driver.Manage().Cookies.DeleteCookieNamed("whoami"); // does not throw if not found
driver.Manage().Cookies.GetCookieNamed("whoami"); // throws if not found

And now, from C# perspective, I am not glad that this API (according specification) is not aligned. I expected that we always can throw if an user is going to execute something with incorrect cookie. We can image that deleting of non-existing cookie is interpreted as success. So technically we can do nothing here, it is as is.

The question now is only about .GetCookieNamed("whoami"): return null or throw. In this context I think it is reasonable even to not change it at all - it will never be aligned. Returning null is not a criminal (current behavior).