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

Fix OAuth refresh attempt when no network available causing full logout (part 2) #25029

Merged
merged 1 commit into from Oct 6, 2023

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Oct 6, 2023

This time for SocketExceptions. I seem to recall looking at this and deciding there was a reason to not catch socket exceptions, but on revisiting it seems sane to do so.

This covers a fail case like reported:

2023-10-06 03:24:17 [verbose]: Request to https://lazer.ppy.sh/oauth/token failed with System.Net.Http.HttpRequestException: No such host is known. (lazer.ppy.sh:443)
2023-10-06 03:24:17 [verbose]: ---> System.Net.Sockets.SocketException (11001): No such host is known.
2023-10-06 03:24:17 [verbose]: at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)

Closes #24890 (again).

…ut (part 2)

This time for `SocketException`s. I seem to recall looking at this and
deciding there was a reason to not catch socket exceptions, but on
revisiting it seems sane to do so.

This covers a fail case like reported:

```
2023-10-06 03:24:17 [verbose]: Request to https://lazer.ppy.sh/oauth/token failed with System.Net.Http.HttpRequestException: No such host is known. (lazer.ppy.sh:443)
2023-10-06 03:24:17 [verbose]: ---> System.Net.Sockets.SocketException (11001): No such host is known.
2023-10-06 03:24:17 [verbose]: at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)
```

Closes ppy#24890 (again).
@peppy peppy added type:online next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! labels Oct 6, 2023
@bdach bdach self-requested a review October 6, 2023 19:52
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

Re-tested with both patches from #24897 (comment), seems fine.

@bdach bdach merged commit 250b911 into ppy:master Oct 6, 2023
15 of 17 checks passed
@peppy peppy deleted the socket-exception-handling branch October 10, 2023 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! size/XS type:online
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Starting osu! without an internet connection shows "Missing password" error
2 participants