Skip to content

Commit

Permalink
Improve handling of exceptions and interrupts during login/shutdown (#…
Browse files Browse the repository at this point in the history
…1430)

* Improve handling of exceptions during login process
* Improve handling of interrupts
* Simplify JDAImpl#closeAudioConnections
  • Loading branch information
MinnDevelopment committed Nov 12, 2020
1 parent 2b87070 commit d6b5083
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 82 deletions.
48 changes: 23 additions & 25 deletions src/main/java/net/dv8tion/jda/api/sharding/DefaultShardManager.java
Expand Up @@ -274,11 +274,6 @@ public void login() throws LoginException
this.queue.remove(shardId);
}
}
catch (final InterruptedException e)
{
LOG.error("Interrupted Startup", e);
throw new IllegalStateException(e);
}
catch (final Exception e)
{
if (jda != null)
Expand Down Expand Up @@ -348,13 +343,17 @@ public void shutdown()
if (this.shards != null)
{
executor.execute(() -> {
this.shards.forEach(jda ->
synchronized (queue) // this makes sure we also get shards that were starting when shutdown is called
{
if (shardingConfig.isUseShutdownNow())
jda.shutdownNow();
else
jda.shutdown();
});
this.shards.forEach(jda ->
{
if (shardingConfig.isUseShutdownNow())
jda.shutdownNow();
else
jda.shutdown();
});
queue.clear();
}
this.executor.shutdown();
});
}
Expand Down Expand Up @@ -396,11 +395,13 @@ protected void enqueueShard(final int shardId)

protected void runQueueWorker()
{
if (shutdown.get())
throw new RejectedExecutionException("ShardManager is already shutdown!");
if (worker != null)
return;
worker = executor.submit(() ->
{
while (!queue.isEmpty())
while (!queue.isEmpty() && !Thread.currentThread().isInterrupted())
processQueue();
this.gatewayURL = null;
synchronized (queue)
Expand Down Expand Up @@ -438,10 +439,12 @@ protected void processQueue()
if (api == null)
api = this.buildInstance(shardId);
}
catch (InterruptedException e)
catch (CompletionException e)
{
//caused by shutdown
LOG.debug("Queue has been interrupted", e);
if (e.getCause() instanceof InterruptedException)
LOG.debug("The worker thread was interrupted");
else
LOG.error("Caught an exception in queue processing thread", e);
return;
}
catch (LoginException e)
Expand All @@ -468,7 +471,7 @@ protected void processQueue()
}
}

protected JDAImpl buildInstance(final int shardId) throws LoginException, InterruptedException
protected JDAImpl buildInstance(final int shardId) throws LoginException
{
OkHttpClient httpClient = sessionConfig.getHttpClient();
if (httpClient == null)
Expand Down Expand Up @@ -561,16 +564,11 @@ protected JDAImpl buildInstance(final int shardId) throws LoginException, Interr
}
}
}
catch (RuntimeException e)
catch (CompletionException e)
{
if (e.getCause() instanceof InterruptedException)
throw (InterruptedException) e.getCause();
//We check if the LoginException is masked inside of a ExecutionException which is masked inside of the RuntimeException
Throwable ex = e.getCause() instanceof ExecutionException ? e.getCause().getCause() : null;
if (ex instanceof LoginException)
throw new LoginException(ex.getMessage());
else
throw e;
if (e.getCause() instanceof LoginException)
throw (LoginException) e.getCause(); // complete() can't throw this because its a checked-exception so we have to unwrap it first
throw e;
}
}

Expand Down
Expand Up @@ -2268,6 +2268,8 @@ public DefaultShardManagerBuilder setMaxBufferSize(int bufferSize)
* If the provided token is invalid.
* @throws IllegalArgumentException
* If the provided token is empty or null. Or the provided intents/cache configuration is not possible.
* @throws net.dv8tion.jda.api.exceptions.ErrorResponseException
* If some other HTTP error occurred.
*
* @return A {@link net.dv8tion.jda.api.sharding.ShardManager ShardManager} instance that has started the login process. It is unknown as
* to whether or not loading has finished when this returns.
Expand Down
Expand Up @@ -96,32 +96,24 @@ public ShardedGateway getShardedGateway(@Nonnull JDA api)
@Override
public void handleResponse(Response response, Request<ShardedGateway> request)
{
try
if (response.isOk())
{
if (response.isOk())
{
DataObject object = response.getObject();

String url = object.getString("url");
int shards = object.getInt("shards");
int concurrency = object.getObject("session_start_limit").getInt("max_concurrency", 1);

request.onSuccess(new ShardedGateway(url, shards, concurrency));
}
else if (response.code == 401)
{
api.shutdownNow();
throw new LoginException("The provided token is invalid!");
}
else
{
request.onFailure(new LoginException("When verifying the authenticity of the provided token, Discord returned an unknown response:\n" +
response.toString()));
}
DataObject object = response.getObject();

String url = object.getString("url");
int shards = object.getInt("shards");
int concurrency = object.getObject("session_start_limit").getInt("max_concurrency", 1);

request.onSuccess(new ShardedGateway(url, shards, concurrency));
}
else if (response.code == 401)
{
api.shutdownNow();
request.onFailure(new LoginException("The provided token is invalid!"));
}
catch (Exception e)
else
{
request.onFailure(e);
request.onFailure(response);
}
}
}.priority().complete();
Expand Down
49 changes: 15 additions & 34 deletions src/main/java/net/dv8tion/jda/internal/JDAImpl.java
Expand Up @@ -17,7 +17,6 @@
package net.dv8tion.jda.internal;

import com.neovisionaries.ws.client.WebSocketFactory;
import gnu.trove.map.TLongObjectMap;
import gnu.trove.set.TLongSet;
import net.dv8tion.jda.api.AccountType;
import net.dv8tion.jda.api.GatewayEncoding;
Expand Down Expand Up @@ -73,7 +72,10 @@
import javax.annotation.Nonnull;
import javax.security.auth.login.LoginException;
import java.util.*;
import java.util.concurrent.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ScheduledExecutorService;
import java.util.stream.Collectors;

public class JDAImpl implements JDA
Expand Down Expand Up @@ -345,32 +347,18 @@ else if (response.isRateLimit())
else if (response.code == 401)
request.onSuccess(null);
else
request.onFailure(new LoginException("When verifying the authenticity of the provided token, Discord returned an unknown response:\n" +
response.toString()));
request.onFailure(response);
}
}.priority();

try
{
DataObject userResponse = login.complete();
if (userResponse != null)
{
getEntityBuilder().createSelfUser(userResponse);
return;
}
shutdownNow();
throw new LoginException("The provided token is invalid!");
}
catch (RuntimeException | Error e)
DataObject userResponse = login.complete();
if (userResponse != null)
{
shutdownNow();
//We check if the LoginException is masked inside of a ExecutionException which is masked inside of the RuntimeException
Throwable ex = e.getCause() instanceof ExecutionException ? e.getCause().getCause() : null;
if (ex instanceof LoginException)
throw new LoginException(ex.getMessage());
else
throw e;
getEntityBuilder().createSelfUser(userResponse);
return;
}
shutdownNow();
throw new LoginException("The provided token is invalid!");
}

public AuthorizationConfig getAuthorizationConfig()
Expand Down Expand Up @@ -735,17 +723,10 @@ public synchronized void shutdownRequester()

private void closeAudioConnections()
{
List<AudioManagerImpl> managers;
AbstractCacheView<AudioManager> view = getAudioManagersView();
try (UnlockHook hook = view.writeLock())
{
managers = view.stream()
.map(AudioManagerImpl.class::cast)
.collect(Collectors.toList());
view.clear();
}

managers.forEach(m -> m.closeAudioConnection(ConnectionStatus.SHUTTING_DOWN));
getAudioManagerCache()
.stream()
.map(AudioManagerImpl.class::cast)
.forEach(m -> m.closeAudioConnection(ConnectionStatus.SHUTTING_DOWN));
}

@Override
Expand Down

0 comments on commit d6b5083

Please sign in to comment.