Make ClientConnectorCertificateError.ssl mean the same as on the base class

[ArockiaRajamanickam]

What do these changes do?

Closes #4099.

ConnectionKey carries two adjacent fields:

is_ssl: bool
ssl: SSLContext | bool | Fingerprint

ClientConnectorError.ssl returns ssl. ClientConnectorCertificateError overrode it to return is_ssl, so the same attribute on the same connection key meant two different things depending on which exception you caught:

ctx = ssl.create_default_context()
key = ConnectionKey(host="example.com", port=443, is_ssl=True, ssl=ctx, ...)

ClientConnectorError(key, OSError(1, "boom")).ssl            # <ssl.SSLContext object>  <- the caller's context
ClientConnectorCertificateError(key, cert_err).ssl           # True                     <- a bool

The override is dropped so the subclass inherits from ClientConnectorError, which is where the attribute is already correct. __str__ now renders ssl is True as default the way the base class does, instead of printing True.

@asvetlov confirmed this back in 2019 ("I think yes. The result of the project evolvement."). This follows the same reasoning as #12136, which restored os_error on this class on Liskov grounds six months ago.

Are there changes in behavior for the user?

Yes, and this is the part worth reviewing carefully.

Anyone reading .ssl on ClientConnectorCertificateError as a boolean now gets whatever was passed to ssl=, which for the default case is True but for a configured client is an SSLContext or a Fingerprint. Code wanting the old boolean should read is_ssl from the connection key. I filed the fragment as breaking rather than bugfix for that reason, and master being 4.0.0a2.dev0 seemed like the right window to correct it.

__str__ also changes for the ssl=True case, from ssl:True to ssl:default, which is what ClientConnectorError already prints.

Three existing assertions had to be flipped, and I want to flag that rather than have it found in review:

# the fixture deliberately sets is_ssl=False, ssl=True
assert err.ssl is False   ->   assert err.ssl is True   # test_ctor and test_pickle
"...ssl:False..."         ->   "...ssl:default..."      # test_str

Those assertions encoded the behaviour being fixed. I would normally treat "the fix requires changing existing tests" as a sign the behaviour is intended, and I checked that here rather than assuming: the issue carries the bug label and a maintainer said plainly that it is one. If that reading is wrong, this should be closed rather than merged.

Also for transparency: the issue is assigned to @asvetlov, from 2019. Nothing has moved on it since, so I took it as open rather than in hand. Happy to close if that is not right.

The redundant host and port overrides on the same class are identical to the base class implementations. I left them alone to keep this diff to the bug.

Is it a substantial burden for the maintainers to support this?

No. The class now inherits three properties instead of overriding two of them incorrectly.

Testing

test_ssl_is_the_same_as_on_the_base_class builds both exceptions from one ConnectionKey holding a real SSLContext and asserts err.ssl is context and err.ssl is base_err.ssl, so is_ssl and ssl cannot be confused by both happening to be True.

Reverting only client_exceptions.py and keeping the tests fails 4 tests in TestClientConnectorCertificateError, including the new one. tests/test_client_exceptions.py is 29 passed / 1 xfailed, and test_client_exceptions.py plus test_client_functional.py together are 318 passed / 27 skipped / 2 xfailed. ruff check reports the same 6 pre-existing findings on these files as master does, none from this change.

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
• Add a new news fragment into the CHANGES/ folder
• Added my name to CONTRIBUTORS.txt

I used an AI assistant while working on this. The reproduction, the check on whether the flipped assertions were load-bearing, and the decision to file this as breaking rather than bugfix are mine, and I ran every result quoted here.

[greptile-apps]

Confidence Score: 4/5

The implementation appears safe to merge, with a non-blocking requirement to document the changed public attribute contract in the client reference documentation.

The exception now consistently exposes ConnectionKey.ssl and the tests cover the meaningful non-boolean case; the remaining issue is that this intentional public API change is documented only in the changelog.

Files Needing Attention: aiohttp/client_exceptions.py and the relevant client reference documentation

_{Reviews (1): Last reviewed commit: "Make ClientConnectorCertificateError.ssl..." | Re-trigger Greptile}

[greptile-apps]

Public API docs remain stale

Removing this override changes ClientConnectorCertificateError.ssl from a boolean to the inherited SSLContext | bool | Fingerprint contract, but no client reference documentation explains the new contract or migration from the former boolean meaning. The repository requires user-visible API changes to be reflected under docs/, rather than only in a changelog fragment.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.02%. Comparing base (20acdf4) to head (a5c98ee).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #13583   +/-   ##
=======================================
  Coverage   99.02%   99.02%           
=======================================
  Files         135      135           
  Lines       50500    50506    +6     
  Branches     2652     2652           
=======================================
+ Hits        50007    50013    +6     
  Misses        370      370           
  Partials      123      123           

Flag	Coverage Δ
Autobahn	22.03% <18.18%> (-0.01%)	⬇️
CI-GHA	98.91% <100.00%> (+<0.01%)	⬆️
OS-Linux	98.68% <100.00%> (+<0.01%)	⬆️
OS-Windows	97.09% <100.00%> (+<0.01%)	⬆️
OS-macOS	97.97% <100.00%> (-0.01%)	⬇️
Py-3.10	98.11% <100.00%> (+<0.01%)	⬆️
Py-3.11	98.34% <100.00%> (+<0.01%)	⬆️
Py-3.12	98.43% <100.00%> (+<0.01%)	⬆️
Py-3.13	98.41% <100.00%> (-0.01%)	⬇️
Py-3.14	98.45% <100.00%> (+<0.01%)	⬆️
Py-3.14t	97.62% <100.00%> (+<0.01%)	⬆️
Py-pypy-3.11	97.39% <100.00%> (+<0.01%)	⬆️
VM-macos	97.97% <100.00%> (-0.01%)	⬇️
VM-ubuntu	98.68% <100.00%> (+<0.01%)	⬆️
VM-windows	97.09% <100.00%> (+<0.01%)	⬆️
cython-coverage	83.08% <ø> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[codspeed-hq]

Merging this PR will not alter performance

✅ 96 untouched benchmarks
⏩ 83 skipped benchmarks¹

---

_{Comparing ArockiaRajamanickam:fix-4099-cert-error-ssl-attr (a5c98ee) with master (20acdf4)}

Footnotes

1. 83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩