ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.4k stars 2.29k forks source link

Playing deleted beatmap throws `APIException` to user #27720

Closed tybug closed 6 months ago

tybug commented 7 months ago

Type

Other

Bug description

cc @ppy/team-client. Should probably be surfaced to the user in a more friendly way.

Originally discussed in https://github.com/ppy/osu/discussions/27716.

2024-03-24 16:19:15 [verbose]: Request to https://osu.ppy.sh/api/v2/beatmaps/4054834/solo/scores failed with System.Net.WebException: NotFound.
2024-03-24 16:19:15 [verbose]: Response was: {"error":null}
2024-03-24 16:19:15 [verbose]: Failing request osu.Game.Online.Solo.CreateSoloScoreRequest (osu.Game.Online.API.APIException: Exception of type 'osu.Game.Online.API.APIException' was thrown.

Screenshots or videos

No response

Version

.

Logs

compressed-logs.zip

smoogipoo commented 7 months ago

I think this probably has to start with osu!web returning an actual error message for when the beatmap isn't found. @ppy/team-web

nanaya commented 7 months ago

Status code 404 is returned when the beatmap isn't found...

smoogipoo commented 7 months ago

Doesn't it seem weird that we have to handle 404s in a very specific way for this one scenario? What if a legitimate 404 actually occurs, like the endpoint becomes inaccessible (for any number of reasons), and we miss it because in that very specific scenario we aren't displaying an error message here?

So there's the question of how should we handle this particular case. In the past we've silently ignored this specific case because it would be too verbose to tell the player that the beatmap doesn't exist online. Curious what @peppy thinks of this.

bdach commented 7 months ago

What if a legitimate 404 actually occurs, like the endpoint becomes inaccessible (for any number of reasons), and we miss it because in that very specific scenario we aren't displaying an error message here?

I think this is a reach, 404 is a legitimate response when a resource isn't found in REST. I'm not sure we should be worried about "false positives" that could technically happen if nginx eats dirt or something and the entirety of the endpoint is not there anymore.

nanaya commented 7 months ago

well, osu-web can return a generic error message ("specified beatmap difficulty (123) couldn't be found"). Currently, most/all requests on missing object return json with null error field.

smoogipoo commented 7 months ago

if nginx eats dirt or something

What about if a change in multiplayer happens that results in us not passing the playlist or room to the API, meaning the playlist or room isn't found? My point is, a 404 is a generic catch all term for any resource. It's not specific to a beatmap not being found, and imo we shouldn't treat it that way.

Or what, we add a specific case for multiplayer where it actually does show NotFound because it's actually an error, but we're expecting that to not be an error in solo?

bdach commented 7 months ago

It's not specific to a beatmap not being found, and imo we shouldn't treat it that way.

Ok yeah that's fair.

nanaya commented 7 months ago

I'll add the message I mentioned above but that said, I'm not sure if client should show the generic error message in this case either...

bdach commented 6 months ago

I guess this is resolved by the web-side change?