Skip to content

Fix timeouts not being applied#685

Merged
sunkup merged 1 commit into
devfrom
684-timeouts-are-not-applied
Sep 4, 2025
Merged

Fix timeouts not being applied#685
sunkup merged 1 commit into
devfrom
684-timeouts-are-not-applied

Conversation

@sunkup

@sunkup sunkup commented Sep 3, 2025

Copy link
Copy Markdown
Member

Purpose

Timeouts were not being applied correctly / at all.

Short description

  • use ktor plugin to apply the timeouts
  • don't set timeout for okhttp directly

https://ktor.io/docs/client-timeout.html

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@sunkup sunkup linked an issue Sep 3, 2025 that may be closed by this pull request
@sunkup sunkup self-assigned this Sep 3, 2025
@sunkup sunkup added the bug label Sep 3, 2025
@sunkup sunkup added this to the 2.4.1 milestone Sep 3, 2025
@sunkup
sunkup requested review from ArnyminerZ and Copilot September 3, 2025 12:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes timeout configuration by moving from OkHttp-specific timeout settings to Ktor's built-in timeout plugin, ensuring timeouts are properly applied across all HTTP client engines.

  • Replaced OkHttp-specific timeout configuration with Ktor's HttpTimeout plugin
  • Removed direct timeout settings from OkHttp engine configuration
  • Added proper timeout values for connect (20s), request (60s), and socket (60s) operations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ArnyminerZ ArnyminerZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. I'd use this opportunity to also get rid of the followRedirects option in the engine, and close #686

@sunkup
sunkup marked this pull request as ready for review September 4, 2025 08:35
@sunkup
sunkup merged commit e90184f into dev Sep 4, 2025
7 checks passed
@sunkup
sunkup deleted the 684-timeouts-are-not-applied branch September 4, 2025 08:35
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.

[ktor] Timeouts are not applied

3 participants