Connection related issue in HTTP client by joamag · Pull Request #48 · hivesolutions/netius

joamag · 2026-04-02T18:20:41Z

Summary by CodeRabbit

Release Notes

Bug Fixes
- Guard against None parsed URL in HTTPProtocol when connection closes during SSL handshake.
- Enhanced detection of transport closing state by considering protocol state.
Tests
- Added comprehensive test coverage for transport closing detection and HTTPProtocol request handling behavior.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c538e58ee5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-02T18:22:55Z

src/netius/base/transport.py

+            self._protocol
+            and hasattr(self._protocol, "is_closing")
+            and self._protocol.is_closing()
+        ):


Don't treat protocol closing as transport already closed

Protocol.close_c() sets _closing = True before calling _close_transport(), and _close_transport() calls Transport.abort(). After this change, Transport.is_closing() returns True whenever self._protocol.is_closing() is true, so abort()/close() now short-circuit and never call self._connection.close(...). This means protocol-initiated closes can leave the underlying connection open (or waiting on peer close), which can stall request teardown and leak sockets under normal close paths.

Useful? React with 👍 / 👎.

coderabbitai · 2026-04-02T18:24:59Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81566700-25a7-484f-b71c-07d882c1f93f

📥 Commits

Reviewing files that changed from the base of the PR and between 21d39f5 and c538e58.

📒 Files selected for processing (5)

CHANGELOG.md
src/netius/base/transport.py
src/netius/clients/http.py
src/netius/test/base/transport.py
src/netius/test/clients/http.py

📝 Walkthrough

Walkthrough

This pull request adds a guard against None parsed URLs in HTTPProtocol.send_request() to handle connection closures during SSL handshake. It updates Transport.is_closing() to detect protocol-level closing state and includes comprehensive test coverage for both changes, along with a changelog entry.

Changes

Cohort / File(s)	Summary
Documentation `CHANGELOG.md`	Added fix entry documenting the guard against None parsed URL in HTTPProtocol.send_request when connection closes during SSL handshake.
Core Logic Changes `src/netius/base/transport.py`, `src/netius/clients/http.py`	Updated Transport.is_closing() to check if protocol is closing; added guard in HTTPProtocol.send_request() to return early when parsed is None.
Unit Tests `src/netius/test/base/transport.py`, `src/netius/test/clients/http.py`	Comprehensive test coverage for Transport.is_closing() across connection states and protocol behaviors; new HTTPProtocolTest class validating send_request() behavior with None parsed state.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A protocol once stumbled in the night,
When SSL's grip turned far too tight,
But now we guard with checks so keen,
No None shall slip between the scene!
With tests that bind each edge case tight,
Our connections dance in pure delight. 🌙

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Connection related issue in HTTP client' is vague and generic. It uses non-descriptive terms that don't convey the specific technical nature of the fix.	Use a more specific title that describes the actual issue being fixed, such as 'Guard against None parsed URL in HTTPProtocol.send_request when connection closes during SSL handshake' or 'Fix HTTP client crash on connection close during SSL handshake'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/closing-connection-issue

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR addresses connection-close edge cases in the Netius HTTP client by guarding request sending when the parsed URL state is unavailable, and by extending transport “closing” detection to include protocol closing state. It also adds unit tests and updates the changelog to document the fix.

Changes:

Add a guard in HTTPProtocol.send_request() to return early when self.parsed is None.
Extend Transport.is_closing() to treat a closing protocol as “closing”.
Add unit tests for the new HTTPProtocol and Transport closing behavior; update CHANGELOG.md.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/netius/clients/http.py`	Adds an early-return guard in `HTTPProtocol.send_request()` when `parsed` is `None`.
`src/netius/base/transport.py`	Extends `Transport.is_closing()` to consider the attached protocol’s closing state.
`src/netius/test/clients/http.py`	Adds tests covering `HTTPProtocol.send_request()` when `parsed` is `None` and `close_c()` behavior.
`src/netius/test/base/transport.py`	Adds tests for `Transport.is_closing()` across connection/protocol state combinations.
`CHANGELOG.md`	Documents the HTTPProtocol guard under Unreleased “Fixed”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-02T18:25:42Z

src/netius/base/transport.py

+        if (
+            self._protocol
+            and hasattr(self._protocol, "is_closing")
+            and self._protocol.is_closing()
+        ):
+            return True


Transport.is_closing() now returns True when the associated protocol is closing. This breaks Transport.abort()/Transport.close() semantics because both methods start with if self.is_closing(): return. Since Protocol.close_c() sets _closing=True and then calls transport.abort(), abort() will now immediately no-op and never close the underlying connection, potentially leaking sockets and preventing clean shutdown. Consider keeping is_closing() based only on connection state, or adjust abort()/close() to bypass the protocol-closing check (e.g., inline-check only _connection/_connection.is_closed() or add a force/ignore_protocol path).

Suggested change

if (

self._protocol

and hasattr(self._protocol, "is_closing")

and self._protocol.is_closing()

):

return True

Copilot · 2026-04-02T18:25:43Z

src/netius/clients/http.py

+        # in case the parsed URL is not available (connection was closed
+        # before the request could be sent) closes the protocol to avoid
+        # errors in the request sending process


The comment says this branch "closes the protocol", but the code only returns early. Either update the comment to match the behavior, or actually close the protocol here (and consider whether any pending callback/request state should be finalized when parsed is None).

Suggested change

# in case the parsed URL is not available (connection was closed

# before the request could be sent) closes the protocol to avoid

# errors in the request sending process

# in case the parsed URL is not available (for example, if the

# connection was closed before the request could be sent) aborts

# sending the request to avoid errors in the request process

Copilot · 2026-04-02T18:25:43Z

src/netius/test/base/transport.py

+    def test_is_closing_protocol_closing(self):
+        connection = netius.Connection()
+        transport = netius.Transport(None, connection)
+
+        protocol = netius.Protocol()
+        protocol._open = True
+        transport._protocol = protocol
+
+        self.assertEqual(transport.is_closing(), False)
+        self.assertEqual(protocol.is_closing(), False)
+
+        protocol._closing = True
+
+        self.assertEqual(transport.is_closing(), True)
+        self.assertEqual(protocol.is_closing(), True)


The new is_closing() tests cover the return value when a protocol is marked closing, but they don't cover the most important behavioral contract: calling protocol.close() should still result in the underlying transport/connection being closed. Adding a regression test for Protocol.close() -> Transport.abort() closing the connection would help catch the interaction introduced by the new protocol-aware is_closing() logic.

chore: initial solution

c538e58

Copilot AI review requested due to automatic review settings April 2, 2026 18:20

joamag self-assigned this Apr 2, 2026

joamag added the enhancement New feature or request label Apr 2, 2026

Copilot started reviewing on behalf of joamag April 2, 2026 18:21 View session

chatgpt-codex-connector bot reviewed Apr 2, 2026

View reviewed changes

Copilot AI reviewed Apr 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connection related issue in HTTP client#48

Connection related issue in HTTP client#48
joamag wants to merge 1 commit intomasterfrom
fix/closing-connection-issue

joamag commented Apr 2, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Apr 2, 2026

Uh oh!

coderabbitai bot commented Apr 2, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

joamag commented Apr 2, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

joamag commented Apr 2, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 2, 2026 •

edited

Loading