GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [pipeline-manager] Send PipelineInteractionConnectFailed when a pipeline connect fails by blp · Pull Request #7860 · feldera/feldera

Conversation

@blp

@blp blp commented Nov 5, 2026

Copy link
Copy Markdown
Member

The api-server sent 503 PipelineInteractionUnreachable both when it could not open a connection to a pipeline and when a request failed part way. Clients cannot safely retry the second case for a request that is not idempotent, so since 7e1d8fd the Python SDK retried neither. A connect timeout right after a pipeline restart then failed HTTP ingress on the first attempt (feldera/cloud#2175, 4 CI failures on amd64).

Area Change
Manager New 503 code PipelineInteractionConnectFailed for SendRequestError::Connect(_), except Disconnected. The overall request Timeout keeps the old code. The two copies of the error mapping are now one function, pipeline_send_error.
API docs New "Connection failed" example on the pipeline-interaction endpoints. openapi.json regenerated.
Python SDK Retries the new code for all requests. Also retries the old code with "Failed to connect to host" in the message, for older managers.
Rust client Retries the new code, as it already did for the old code with the connect message.
Test helper wait_for_pipeline_reachable accepts both codes.

Tests: new unit tests in the manager, the Python SDK and the Rust client. Each one fails when the fix is reverted.

Fixes feldera/cloud#2175.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@blp
[pipeline-manager] Send PipelineInteractionConnectFailed when a pipel…
6c0bf4b
…ine connect fails.

The api-server sent 503 PipelineInteractionUnreachable both when it could
not open a connection to a pipeline and when a request failed part way.
Clients cannot safely retry the second case for a request that is not
idempotent, so since 7e1d8fd the Python SDK retried neither. A connect
timeout right after a pipeline restart then failed HTTP ingress on the
first attempt.

A connect failure now gets its own code, PipelineInteractionConnectFailed.
The request was not sent, so the Python SDK and the Rust client retry it
for all requests. Both also retry the old code with the "Failed to connect
to host" message, for managers that predate this change.

Fixes feldera/cloud#2175.
@blp blp added bug Something isn't working Pipeline manager Pipeline manager (API, API server, runner, compiler server) API Distributed system APIs python-sdk Issues related to the feldera python sdk rust Pull requests that update Rust code labels Nov 5, 2026
@blp blp self-assigned this Nov 5, 2026
@blp
blp requested a review from mihaibudiu November 5, 2026 18:54
mihaibudiu
mihaibudiu approved these changes Nov 5, 2026
View reviewed changes
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 5, 2026
View reviewed changes
Comment thread crates/pipeline-manager/src/runner/interaction.rs
error: format_disconnected_error_message(e),
}
}
SendRequestError::Connect(_) => RunnerError::PipelineInteractionConnectFailed {

@mythical-fred-oss mythical-fred-oss Bot Nov 5, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

pitfall (medium): "Retry is safe" depends on how awc works inside. I checked awc 3.7.0. h1proto.rs returns ConnectError::Disconnected after it writes the request (via ok_or(ConnectError::Disconnected)??), which is why that variant has to stay excluded. Every other ConnectError comes from the connector or the pool before any bytes are sent. If a future awc bump returns some other ConnectError after the request is sent, this would silently turn into a double-apply. The unit test only uses hand-built errors, so it can't catch that. Please add a socket-level test that pins it down (diff in the top-level comment).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@blp blp Nov 5, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Added both tests in 03cc63f, as test_streaming_proxy_connection_refused_is_connect_failed and test_streaming_proxy_close_after_accept_is_unreachable. I checked that each one fails when the classification is broken. If the Disconnected arm is removed, the close-after-accept test fails. If all Connect errors map to Unreachable, the connection-refused test fails.

-- Claude Code, replying at @blp's request.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mythical-fred-oss

mythical-fred-oss Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

What I ran

Check Result
cargo test -p feldera-rest-api (unit + tests/retry.rs) ✅ pass
cargo test -p pipeline-manager --lib send_error ✅ pass
uv run pytest tests/unit (python) ✅ 425 passed
pre-commit run --files <changed> (incl. OpenAPI regen, clippy) ✅ pass
Manually checked awc 3.7.0: which ConnectError variants can occur after the request is sent only Disconnected, which is excluded ✅

No gate fails: the PR has tests, a changelog entry, no new dependencies and no unsafe.

Missing (medium): no test runs the classification against a real socket. send_error_connect_failure_is_distinct only covers hand-built Connect(Timeout). The most common real case, connection refused (ConnectError::Io), is not tested. Neither is the safety-critical case: a peer that accepts the request and then drops the connection must not get the retryable code. I added both tests to runner::interaction::tests, and they pass on this branch:

+    /// A refused connection reaches the client as the retryable code.
+    #[actix_web::test]
+    async fn test_streaming_proxy_connection_refused_is_connect_failed() {
+        setup();
+        // Bind and drop a listener to get a port with nothing listening on it.
+        let port = std::net::TcpListener::bind("127.0.0.1:0")
+            .unwrap().local_addr().unwrap().port();
+        let url = format!("http://127.0.0.1:{port}/ingress/t0");
+        let (req, payload) = test_get_request(&[]);
+        let err = streaming_proxy(&awc::Client::default(), &url, "test-pipeline",
+            &req, payload, Duration::from_secs(10)).await.unwrap_err();
+        assert_eq!(err.error_code(), "PipelineInteractionConnectFailed");
+    }
+
+    /// A pipeline that accepts the request and then closes the connection may
+    /// have applied it, so the client must not get the retryable code.
+    #[actix_web::test]
+    async fn test_streaming_proxy_close_after_accept_is_unreachable() {
+        setup();
+        let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
+        let port = listener.local_addr().unwrap().port();
+        let server = std::thread::spawn(move || {
+            use std::io::Read;
+            let (mut stream, _) = listener.accept().unwrap();
+            let _ = stream.read(&mut [0u8; 1024]);
+            // Dropping the stream closes the connection without a response.
+        });
+        let url = format!("http://127.0.0.1:{port}/ingress/t0");
+        let (req, payload) = test_get_request(&[]);
+        let err = streaming_proxy(&awc::Client::default(), &url, "test-pipeline",
+            &req, payload, Duration::from_secs(10)).await.unwrap_err();
+        server.join().unwrap();
+        assert_eq!(err.error_code(), "PipelineInteractionUnreachable");
+    }

Otherwise LGTM.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@blp
[pipeline-manager] Test connect-failure classification against real s…
03cc63f
…ockets.

A refused connection must give PipelineInteractionConnectFailed. A peer
that reads the request and then closes the connection must give
PipelineInteractionUnreachable, because it may have applied the request.
These tests catch an awc change that reports a post-send failure as some
other ConnectError.

Signed-off-by: Ben Pfaff <[email protected]>
@blp
blp enabled auto-merge November 5, 2026 19:24
@blp
blp disabled auto-merge November 5, 2026 19:33
@blp
blp added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit b5eb221 Nov 5, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.

@blp
blp deleted the manager-connect-failed-code branch November 5, 2026 23:29
This was referenced Nov 6, 2026
[pipeline-manager] Retry opening the connection to a pipeline for up to 35 s #7868
Draft
Define which layer retries each kind of request failure #7879
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@mihaibudiu mihaibudiu mihaibudiu approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@blp blp

Labels

API Distributed system APIs bug Something isn't working Pipeline manager Pipeline manager (API, API server, runner, compiler server) python-sdk Issues related to the feldera python sdk rust Pull requests that update Rust code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@blp @mihaibudiu
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.