Skip to content

Conversation

@sunkup
Copy link
Member

@sunkup sunkup commented Sep 3, 2025

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 Something isn't working 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
Copy link
Contributor

Copilot AI left a comment

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.

Copy link
Member

@ArnyminerZ ArnyminerZ left a comment

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

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ktor] Timeouts are not applied

2 participants