[PR #394] [MERGED] enable / standardize client request timeouts #431

Closed
opened 2026-02-27 20:24:43 +03:00 by kerem · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/394
Author: @eladyn
Created: 3/9/2023
Status: Merged
Merged: 3/12/2023
Merged by: @ramsayleung

Base: masterHead: client_request_timeouts


📝 Commits (3)

  • 16c7689 reuse agent for ureq requests
  • ebc7d37 set 10 second timeout for ureq and reqwest
  • db46fff update changelog

📊 Changes

3 files changed (+35 additions, -9 deletions)

View changed files

📝 CHANGELOG.md (+3 -0)
📝 rspotify-http/src/reqwest.rs (+13 -1)
📝 rspotify-http/src/ureq.rs (+19 -8)

📄 Description

Description

This sets the same timeout duration for reqwest and ureq http clients.

Motivation and Context

github.com/ramsayleung/rspotify@6d304de632/rspotify-model/src/auth.rs (L79-L80)

This mentions a maximum request time of 10 seconds, but AFAICT, these timeouts aren't specified anywhere. By default, reqwest does not have a timeout at all, ureq has a 30 second connect timeout.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How has this been tested?

The provided non-modifying tests.

Is this change properly documented?

I hope so. :) Please point me to additional documentation changes that are necessary, if there are any.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/ramsayleung/rspotify/pull/394 **Author:** [@eladyn](https://github.com/eladyn) **Created:** 3/9/2023 **Status:** ✅ Merged **Merged:** 3/12/2023 **Merged by:** [@ramsayleung](https://github.com/ramsayleung) **Base:** `master` ← **Head:** `client_request_timeouts` --- ### 📝 Commits (3) - [`16c7689`](https://github.com/ramsayleung/rspotify/commit/16c768910369dbbcc173e0874480f8c66fb2371d) reuse agent for ureq requests - [`ebc7d37`](https://github.com/ramsayleung/rspotify/commit/ebc7d37842207a538df0ce88d692e41aae7dd482) set 10 second timeout for ureq and reqwest - [`db46fff`](https://github.com/ramsayleung/rspotify/commit/db46fffab9909cdff04052bdc04c81bb6673dbf7) update changelog ### 📊 Changes **3 files changed** (+35 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+3 -0) 📝 `rspotify-http/src/reqwest.rs` (+13 -1) 📝 `rspotify-http/src/ureq.rs` (+19 -8) </details> ### 📄 Description ## Description This sets the same timeout duration for `reqwest` and `ureq` http clients. ## Motivation and Context https://github.com/ramsayleung/rspotify/blob/6d304de632a84f79c35cb9ff4a0e66be238cf205/rspotify-model/src/auth.rs#L79-L80 This mentions a maximum request time of 10 seconds, but AFAICT, these timeouts aren't specified anywhere. By default, `reqwest` does not have a timeout at all, `ureq` has a 30 second `connect` timeout. ## Type of change Please delete options that are not relevant. - [X] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ## How has this been tested? The provided non-modifying tests. ## Is this change properly documented? I hope so. :) Please point me to additional documentation changes that are necessary, if there are any. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-02-27 20:24:43 +03:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/rspotify#431
No description provided.