Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase timeout when using proxy #436

Closed
wants to merge 1 commit into from

Conversation

arch-btw
Copy link
Contributor

@arch-btw arch-btw commented Sep 8, 2023

On large playlists and/or high-latency networks there is not enough time for all album covers to finish downloading.

WARN psst_gui::delegate] failed to fetch image: https://i.scdn.co/image/... Connection Failed: tls connection init failed: Resource temporarily unavailable (os error 11)

Thank you 😃

On large playlists and/or high-latency networks there is not enough time for all album covers to download:

> WARN  psst_gui::delegate] failed to fetch image: https://i.scdn.co/image/... Connection Failed: tls connection init failed: Resource temporarily unavailable (os error 11)
@jacksongoode
Copy link
Collaborator

Thanks for the PR! @Insprill do you think this would cause any side effects? I think it's within reason for long playlists.

@@ -5,9 +5,9 @@ use quick_protobuf::{BytesReader, MessageRead, MessageWrite, Writer};

use crate::error::Error;

pub const NET_CONNECT_TIMEOUT: Duration = Duration::from_millis(8 * 1000);
pub const NET_CONNECT_TIMEOUT: Duration = Duration::from_millis(16 * 1000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the connection timeout need to be increased?

@arch-btw
Copy link
Contributor Author

Hello, thank you both!
Yes you're right, only NET_IO_TIMEOUT needs an increase for this to work, not the other one.

@jacksongoode
Copy link
Collaborator

@Insprill I can remove the change from NET_CONNECT_TIMEOUT and we can merge?

@Insprill
Copy link
Collaborator

Insprill commented Oct 4, 2023

Yeah that'd be good, I'd like to get this merged too.

@jacksongoode jacksongoode mentioned this pull request Oct 5, 2023
@jacksongoode
Copy link
Collaborator

See #442

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