Skip to content

fix: Allow Ctrl+C to work while waiting on OAuth flow#7

Merged
jpage-godaddy merged 2 commits into
mainfrom
sigterm-not-working
May 29, 2026
Merged

fix: Allow Ctrl+C to work while waiting on OAuth flow#7
jpage-godaddy merged 2 commits into
mainfrom
sigterm-not-working

Conversation

@jpage-godaddy
Copy link
Copy Markdown
Collaborator

This pkce.rs was doing a wait for a callback in such a way where SIGTERM was not working (like when hitting Ctrl+C). Fix.

@jpage-godaddy jpage-godaddy changed the title Fix Ctrl+C breakage fix: Allow Ctrl+C to work while waiting on OAuth flow May 28, 2026
This `pkce.rs` was doing a wait for a callback in such a way where `SIGTERM` was not working (like when hitting `Ctrl+C`). Fix.
The async rewrite of wait_for_callback changed accept-error handling
from fail-fast (propagate the error) to retry-on-continue. With no
delay, a persistent accept() failure such as file-descriptor
exhaustion turns the loop into a tight CPU spin that runs until the
120s timeout fires.

Sleep 50ms before retrying. The sleep is an await point, so it keeps
the original PR's Ctrl+C cancellation behavior intact while bounding
the retry rate.
@jgowdy-godaddy
Copy link
Copy Markdown
Collaborator

Pushed a small follow-up commit (876a0fe) on top of this branch.

Why: The async rewrite here is correct and fixes the Ctrl+C hang. In the same lines, the accept-error path also changed from fail-fast to retry-on-continue:

let (mut stream, _) = match listener.accept().await {
    Ok(conn) => conn,
    Err(_) => continue,
};

Retrying is the right call for transient, per-connection errors (e.g. ECONNABORTED). The gap is the persistent case: under fd exhaustion (EMFILE/ENFILE), accept() returns Err immediately and keeps doing so, turning the loop into a tight CPU spin that burns a core until the 120s timeout fires. The pre-PR code propagated that error instead of looping.

What changed: a 50ms backoff before retrying.

Err(_) => {
    tokio::time::sleep(Duration::from_millis(50)).await;
    continue;
}

This bounds the retry rate while still recovering if the condition clears. tokio::time::sleep is an await point, so it preserves the exact Ctrl+C cancellation behavior this PR delivers — the flow still cancels promptly mid-backoff.

Verification: full suite green on the amended branch — cargo fmt --all --check, cargo clippy --all-targets -- -D warnings, RUSTDOCFLAGS='-D warnings' cargo doc --no-deps, cargo test --all-targets (0 failures), cargo test --doc, and cargo rustdoc --lib -- -W missing-docs (0 missing-docs).

@jpage-godaddy
Copy link
Copy Markdown
Collaborator Author

Additional change from @jgowdy-godaddy looks good to me as well.

@jpage-godaddy jpage-godaddy merged commit 2b6d10e into main May 29, 2026
2 checks passed
@jpage-godaddy jpage-godaddy deleted the sigterm-not-working branch May 29, 2026 00:07
jpage-godaddy pushed a commit that referenced this pull request Jun 1, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.1.2](cli-engine-v0.1.1...cli-engine-v0.1.2)
(2026-06-01)


### Features

* Allow hard-coded redirect URL
([#9](#9))
([e24dc24](e24dc24))
* Fix keychain issues and add fs fallback
([#10](#10))
([853a98a](853a98a))


### Bug Fixes

* Allow Ctrl+C to work while waiting on OAuth flow
([#7](#7))
([2b6d10e](2b6d10e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants