Skip to content

fix: try multiple times to connect to an AP#87

Merged
devgianlu merged 1 commit intodevgianlu:masterfrom
aykevl:fix-ap-connection-refused
Sep 10, 2024
Merged

fix: try multiple times to connect to an AP#87
devgianlu merged 1 commit intodevgianlu:masterfrom
aykevl:fix-ap-connection-refused

Conversation

@aykevl
Copy link
Copy Markdown
Contributor

@aykevl aykevl commented Sep 9, 2024

I sometimes get "connection refused" for no apparent reason, even though the same AP works at other times. So when that happens, try again with a different AP.

I don't know why this happens, but with this patch the connecting code is a bit more robust. I've verified that it works: it connects to the next in the list which usually works.

I sometimes get "connection refused" for no apparent reason, even though
the same AP works at other times. So when that happens, try again with a
different AP.

I don't know why this happens, but with this patch the connection code
should be a bit more robust.
Comment thread ap/ap.go
for {
attempts++
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
defer cancel()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This defer will trigger only when returning from the function therefore we might have up to 6 contexts running simultaneosly. This shouldn't be an issue, just saying.

Comment thread ap/ap.go
return fmt.Errorf("failed to connect to AP %v: %e", addr, err)
}
// Try again with a different AP.
log.Warnf("failed to connect to AP %v (error: %v), retrying with a different AP", addr, err)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You could use .WithError() error here, but also just a note.

@devgianlu
Copy link
Copy Markdown
Owner

devgianlu commented Sep 10, 2024

This seems to be an issue with Spotify infrastructure. It has been reported by many users: https://community.volumio.com/t/new-2023-spotify-plugin/63381/378

Since this is a bit urgent I am merging anyway, feel free to open another PR to take care of the comments above.

@devgianlu devgianlu merged commit 0ad7f6a into devgianlu:master Sep 10, 2024
@aykevl aykevl deleted the fix-ap-connection-refused branch September 10, 2024 10:02
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.

2 participants