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

OpenUrlExternally() can crash game on broken configurations #24696

Closed
SheepCommander opened this issue Sep 1, 2023 · 14 comments · Fixed by ppy/osu-framework#5989
Closed

OpenUrlExternally() can crash game on broken configurations #24696

SheepCommander opened this issue Sep 1, 2023 · 14 comments · Fixed by ppy/osu-framework#5989
Labels
good first issue A good place to start contributing to this project without going too deep. osu!framework issue Can't resolve this without changes to osu!framework. type:reliability

Comments

@SheepCommander
Copy link

SheepCommander commented Sep 1, 2023

Type

Crash to desktop

Bug description

Consistently if I click "View in Browser" three times within a certain time frame, Osu will freeze and crash.

Waiting long enough between clicks will not cause this crash, but an impatient me accidentally discovered it anyways.

Possibly related is that my default browser is the ungoogled-chromium-windows project, specifically a Portable version if that matters.

Screenshots or videos

image

2023-09-01.01-02-19.mp4

Version

2023.815.0-lazer

Logs

network.log
performance.log
runtime.log
updater.log
database.log
input.log

@SheepCommander SheepCommander changed the title Click "Open in browser" 3 times to crash! Click "View in browser" 3 times to crash! Sep 1, 2023
@bdach
Copy link
Collaborator

bdach commented Sep 1, 2023

Um.

2023-09-01 08:04:45 [error]: System.ComponentModel.Win32Exception (0x800401F5): An error occurred trying to start process 'https://osu.ppy.sh/beatmapsets/908#osu/7036' with working directory 'C:\Users\zachi\AppData\Local\osulazer\app-2023.815.0'. Application not found
2023-09-01 08:04:45 [error]: at System.Diagnostics.Process.StartWithShellExecuteEx(ProcessStartInfo startInfo)
2023-09-01 08:04:45 [error]: at System.Diagnostics.Process.Start()
2023-09-01 08:04:45 [error]: at System.Diagnostics.Process.Start(ProcessStartInfo startInfo)
2023-09-01 08:04:45 [error]: at osu.Framework.Platform.DesktopGameHost.openUsingShellExecute(String path)
2023-09-01 08:04:45 [error]: at osu.Framework.Platform.DesktopGameHost.OpenUrlExternally(String url)
2023-09-01 08:04:45 [error]: at osu.Game.Graphics.UserInterface.ExternalLinkButton.OnClick(ClickEvent e)

I think this is going to be something with your machine. Misconfigured shell associations or something like that.

I guess we can try-catch to make it not-crash, but it won't work because your registry seems broken.

@bdach bdach changed the title Click "View in browser" 3 times to crash! OpenUrlExternally() can crash game on broken configurations Sep 1, 2023
@bdach bdach added osu!framework issue Can't resolve this without changes to osu!framework. good first issue A good place to start contributing to this project without going too deep. type:reliability labels Sep 1, 2023
@ppy-sentryintegration
Copy link

Sentry issue: OSU-FHV

@SheepCommander
Copy link
Author

Misconfigured shell and broken registry?

Damn Windows. This is a fresh install ;-;

@bdach
Copy link
Collaborator

bdach commented Sep 2, 2023

Pausibly related is that my default browser is the ungoogled-chromium-windows project, specifically a Portable version if that matters.

Whatever you did there probably broke it. If it's on some drive that's not always available, this crash is gonna happen when you try to open websites via lazer when it's not - because the registry associates the handler for URLs with the portable install, so if it's not there for it to use, nothing can work sanely anymore.

@SheepCommander
Copy link
Author

SheepCommander commented Sep 3, 2023

My entire computer is on a single drive, but yep def has to do with the portable install which I've deleted and re-downloaded a couple times

@peppy
Copy link
Member

peppy commented Sep 3, 2023

Installing any browser should fix the issue

@SheepCommander
Copy link
Author

Oh btw it indeed does, I forgot to mention

@bdach
Copy link
Collaborator

bdach commented Sep 6, 2023

Reopening so that the reliability fix is actually made.

@bdach bdach reopened this Sep 6, 2023
@Seligmann
Copy link

Not sure if the answer to this is already realized or not, but is the issue that the removable media wasn't plugged in that the portable browser was on when trying to OpenUrlExternally() , or is the file association not able to be built as intended if it's on a separate (and or removable) drive from Osu?

@peppy
Copy link
Member

peppy commented Sep 10, 2023

The issue is that we need to not crash on a failure of this function. It just needs a try-catch.

@yesseruser
Copy link
Contributor

I'd like to add the try-catch.
Could you assign me please?

@Seligmann
Copy link

Do we need to be assigned? I actually already added the try-catch and made a demo video, but when I tried to open a PR to merge my feature branch into main, I don't have permissions. From looking at other PRs, I need to fork OSU and go about things that way.

@bdach
Copy link
Collaborator

bdach commented Sep 12, 2023

This issue is so simple that it should not require assignments. First PR probably wins, if it's not doing anything egregiously wrong.

@yesseruser
Copy link
Contributor

^ Created a PR.
ppy/osu-framework#5989

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good place to start contributing to this project without going too deep. osu!framework issue Can't resolve this without changes to osu!framework. type:reliability
Projects
None yet
5 participants