Skip to content

feat: add support for making HTTP requests#4

Closed
jjhafer wants to merge 4 commits intosilvermine:masterfrom
jjhafer:jhafer/implement_http_requests
Closed

feat: add support for making HTTP requests#4
jjhafer wants to merge 4 commits intosilvermine:masterfrom
jjhafer:jhafer/implement_http_requests

Conversation

@jjhafer
Copy link
Contributor

@jjhafer jjhafer commented Mar 12, 2026

Includes the following features:

  • A dynamic 'allowlist' which can be configured at initialization and during an app session.

  • Retry support.

  • See README for more information.

@jjhafer jjhafer force-pushed the jhafer/implement_http_requests branch from afc9918 to 25e3f31 Compare March 12, 2026 19:38
@jjhafer jjhafer force-pushed the jhafer/implement_http_requests branch from 25e3f31 to f1dce9d Compare March 12, 2026 22:06
@jjhafer jjhafer marked this pull request as ready for review March 12, 2026 22:09
@jjhafer jjhafer requested a review from velocitysystems March 12, 2026 22:09
@jjhafer
Copy link
Contributor Author

jjhafer commented Mar 12, 2026

@onebytegone I'll ask @velocitysystems to do a review, but if you want to look at the API or tests to get a feel for how we'll use it, please feel free.

src/client.rs Outdated
// (max_attempts >= 1, so attempt 0 always executes and sets last_result).
match last_result.unwrap() {
Ok(mut resp) => {
resp.retry_count = max_attempts - 1;

Choose a reason for hiding this comment

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

The code after this for loop appears to be unreachable: on the final iteration (attempt + 1 == max_attempts), should_retry is false, so the match always falls through to the _ => arm which returns directly. That means last_result.unwrap() can never actually execute.

Would it be worth restructuring the loop to make the control flow explicit — e.g., a loop { ... break } that eliminates the Option wrapper entirely? That would remove the misleading "Safety" comment and make it clear to future readers that every path returns from inside the loop.

Comment added by AI model claude-opus-4-6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, updated. What do you think of the change?

Includes the following features:

* A dynamic 'allowlist' which can be configured at
initialization and during an app session.

* Retry support.

See README for more information.
@jjhafer jjhafer force-pushed the jhafer/implement_http_requests branch from f1dce9d to 48b21b0 Compare March 13, 2026 14:28
@jjhafer jjhafer requested a review from velocitysystems March 13, 2026 14:29
@jjhafer jjhafer closed this Mar 13, 2026
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.

3 participants