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

new Core(params) Throwing when Discord is not running (java-impl branch) #76

Open
Desoroxxx opened this issue Feb 12, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@Desoroxxx
Copy link
Contributor

When Discord is not running new Core(params) thows a FileNotFoundException.
I use this: params.setFlags(CreateParams.Flags.NO_REQUIRE_DISCORD);
So I wasn't really excepting this to happen, I expected a debug log saying that it could not find Discord so it was either not present or not running.

java.lang.RuntimeException: java.io.FileNotFoundException: \\?\pipe\discord-ipc-0 (The system cannot find the file specified)
	at de.jcm.discordgamesdk.Core.<init>(Core.java:126)
	at dev.redstudio.hh.handlers.DiscordGameSdkHandler.lambda$init$0(DiscordGameSdkHandler.java:34)
	at java.base/java.lang.VirtualThread.run(VirtualThread.java:309)
Caused by: java.io.FileNotFoundException: \\?\pipe\discord-ipc-0 (The system cannot find the file specified)
	at java.base/java.io.RandomAccessFile.open0(Native Method)
	at java.base/java.io.RandomAccessFile.open(RandomAccessFile.java:356)
	at java.base/java.io.RandomAccessFile.<init>(RandomAccessFile.java:273)
	at java.base/java.io.RandomAccessFile.<init>(RandomAccessFile.java:223)
	at java.base/java.io.RandomAccessFile.<init>(RandomAccessFile.java:137)
	at de.jcm.discordgamesdk.impl.channel.WindowsDiscordChannel.<init>(WindowsDiscordChannel.java:22)
	at de.jcm.discordgamesdk.Core.getDiscordChannel(Core.java:56)
	at de.jcm.discordgamesdk.Core.<init>(Core.java:119)
	... 2 more
	```
@JnCrMx JnCrMx added the bug Something isn't working label Feb 12, 2024
@JnCrMx
Copy link
Owner

JnCrMx commented Feb 12, 2024

The NO_REQUIRE_DISCORD was originally used in the native version of this library to prevent the application from closing (uncleanly, via a native exit call) and attempting to restart it through Discord in case Discord was not running.
Even in the native version, using this flag would still cause exceptions if Discord was not running (but in runCallbacks), because invoking any functions of the SDK fails without Discord being present.

While I personally think that throwing an exception is the most useful thing to do in this case, the documentation is certainly lacking, and the behavior is unexpected.

My idea to address this would be to deprecate CreateParams.Flags.NO_REQUIRE_DISCORD and instead provide a Core.isDiscordRunning method.

The alternatives would be to silently fail if the flag is set and Discord is not running, or to follow the original behavior and fail with a GameSDKException in runCallbacks().
However, none of the two alternative options seem good to me (I'd rather have the user simply catch the exceptions and deal with them as they see fit for their application).

What do you think?

@Desoroxxx
Copy link
Contributor Author

What do you think?

Maybe if NO_REQUIRE_DISCORD is being used, it could just log a warn or a debug log explaining what happened and saying that it most likely means that Discord is not running.

I like the idea of a Core.isDiscordRunning method if it is static, as it would allow more things that just handling this more cleanly.

@letorbi
Copy link
Contributor

letorbi commented Apr 5, 2024

I also stumbled upon this exception when I developed the improved java channel setup (#61), but was not sure how to handle it. Therefore I just let the exception happen, if a pipe or socket cannot be found. The most common case, where this exception happens, is when the app the is started before Discord itself has been started.

The biggest problem is, that an app, which uses discord-game-sdk4j won't try to reconnect to Discord after the channel setup failed once. So the app has to be restarted after Discord has been started in order to update the state etc.

My idea is to enhance the WindowsDiscordChannel and UnixDiscordChannel channel classes so that they try to connect to a socket/pipe before each read/write command, if no connection has been established already.

However, I am not sure whether this is a good idea or not, because it won't prevent the exception in the setup phase and might even trigger more with each command. I think an option would be to just emit a warning if NO_REQUIRE_DISCORD or something similar is set.

What do you think?

Beside that: I don't really understand how Core.isDiscordRunning could replace NO_REQUIRE_DISCORD, since the former would simply show whether Discord is running or not, while the latter indicates whether Discord is required or optional for the app. Or am I getting something wrong here?

@JnCrMx
Copy link
Owner

JnCrMx commented Apr 5, 2024

Beside that: I don't really understand how Core.isDiscordRunning could replace NO_REQUIRE_DISCORD, since the former would simply show whether Discord is running or not, while the latter indicates whether Discord is required or optional for the app. Or am I getting something wrong?

I think the question ultimately comes down to how much the library wants to handle and how much the application should handle.

Naturally, none of our features are going to work if Discord is not running. The NO_REQUIRE_DISCORD flag originally simply prevents the app from getting terminated if Discord cannot be found. Every runCallbacks would still result in an error, as it should in such a situation, and every other operation simply won't execute.

In that sense NO_REQUIRE_DISCORD and Core.isDiscordRunning would fulfill the same purpose, with the only difference being that with Core.isDiscordRunning the application itself can (and has to) decide how to handle the situation.

The biggest problem is, that an app, which uses discord-game-sdk4j won't try to reconnect to Discord after the channel setup failed once. So the app has to be restarted after Discord has been started in order to update the state etc.

My idea is to enhance the WindowsDiscordChannel and UnixDiscordChannel channel classes so that they try to connect to a socket/pipe before each read/write command, if no connection has been established already.

However, I am not sure whether this is a good idea or not, because it won't prevent the exception in the setup phase and might even trigger more with each command. I think an option would be to just emit a warning if NO_REQUIRE_DISCORD or something similar is set.

In my opinion, it would be best to introduce an AUTO_RECONNECT flag, that can be set in order to cause the library to automatically attempt to reconnect on each command.
Some apps might desire it (since it makes stuff easier) while others might prefer avoiding the overhead of opening the socket each time.

I would also change the NO_REQUIRE_DISCORD to simply suppress all exceptions related to Discord not being preset. This would deviate from the behavior of the original SDK for runCallbacks, but it would be more consistent imo (compared to failing sometimes and sometimes now). However, then I would also rename it to something that better reflects its behavior (otherwise people might believe the library can work without Discord). I'd be happy to hear suggestions for a new name 😄

The behavior I propose would be the following:

  • Discord not available

The library does not work and throws an exception on the construction of the Core.

  • Discord dies (and optionally restarts)

Once Discord dies, the library stops working and throws exceptions on every call. If Discord is running again, it will not attempt to reconnect. However, the application can manually cause a reconnect, either by recreating the Core or by calling a (new) reconnect() method on it.

  • AUTO_RECONNECT + Discord not available

The library tries to connect on each command and on constructing the Core and fails (with an exception) if that is not possible.

  • AUTO_RECONNECT + Discord running then died and restarted

The library works as excepted while Discord is running. While it is dead all calls with fail and throw exceptions. Once Discord is up again, all calls will work as excepted again (event handlers will automatically reconnect). For this case it might be interesting to queue the commands while Discord is dead (so they don't fail yet) and send them once Discord is back. However, this introduces complexity and we would need some kind of timeout. So I don't know if that is a good idea.

  • NO_REQUIRE_DISCORD + Discord not available

The library does not work, but all exceptions are suppressed.

  • NO_REQUIRE_DISCORD + Discord dies (and optionally restart)

Same as without NO_REQUIRE_DISCORD, but there are no exceptions thrown no matter what.

  • NO_REQUIRE_DISCORD + AUTO_RECONNECT + Discord not available, but becomes available later on

The library does not work, all exceptions are suppressed, but as soon as Discord is up, the library will work again. Here we also might choose to queue commands.

  • NO_REQUIRE_DISCORD + AUTO_RECONNECT + Discord running then died and restarted

Same as without NO_REQUIRE_DISCORD, but there are no exceptions thrown no matter what.

@letorbi
Copy link
Contributor

letorbi commented Apr 9, 2024

I've tried to implement some kind of auto-reconnect functionality yesterday, but all my efforts were fruitless, because I was not able to resolve all channel related dependencies. For example some managers require an open channel and also some other components don't seem to like it when the channel is closed and then re-opened.

In the end I came to the conclusion that the best way to handle a disconnect is to destroy the old core and create a new core once Discord is available again. Since this needs a way to determine whether Discord is running or not, I've created the pull request #80, which adds a isDiscordRunning() method as suggested by JnCrMx to the core. All details can be found in the description of the pull request.

@Desoroxxx
Copy link
Contributor Author

So I started using Core#isDiscordRunning which is great, now I can reconnect to Discord if it dies for whatever reason.

Only thing left would be a way to ignore exceptions that are created when Discord doesn't exist and it would be perfect!

@JnCrMx
Copy link
Owner

JnCrMx commented May 29, 2024

That's cool!

Then I'll look into a suppressExceptions flag or something in that direction next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants