Improve database error messages

- Don't log "database is closed" when query tries to execute after database close
- Log better error help when mysql fails to connect

Affects issues:
- Fixed #2499
This commit is contained in:
Aurora Lahtela 2022-09-02 18:49:36 +03:00
parent 0c59659a4e
commit cbc880c1d3
11 changed files with 85 additions and 22 deletions

View File

@ -32,7 +32,6 @@ import javax.inject.Singleton;
@Singleton
public class BukkitDBSystem extends DBSystem {
private final PlanConfig config;
@Inject
public BukkitDBSystem(
@ -42,8 +41,7 @@ public class BukkitDBSystem extends DBSystem {
PlanConfig config,
PluginLogger logger
) {
super(locale, sqLiteDB, logger);
this.config = config;
super(config, locale, sqLiteDB, logger);
databases.add(mySQLDB);
databases.add(sqLiteDB.usingDefaultFile());

View File

@ -0,0 +1,29 @@
/*
* This file is part of Player Analytics (Plan).
*
* Plan is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License v3 as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Plan is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Plan. If not, see <https://www.gnu.org/licenses/>.
*/
package com.djrapitops.plan.exceptions.database;
/**
* Database is closed and exception is needed to stop execution, not to be logged.
*
* @author AuroraLS3
*/
public class DBClosedException extends DBOpException {
public DBClosedException(String message) {
super(message);
}
}

View File

@ -98,7 +98,7 @@ public class PlanConfig extends Config {
}
public boolean isTrue(Setting<Boolean> setting) {
return get(setting);
return Boolean.TRUE.equals(get(setting));
}
public boolean isFalse(Setting<Boolean> setting) {

View File

@ -16,7 +16,7 @@
*/
package com.djrapitops.plan.storage.database;
import com.djrapitops.plan.exceptions.EnableException;
import com.djrapitops.plan.exceptions.database.DBClosedException;
import com.djrapitops.plan.storage.database.transactions.Transaction;
import com.djrapitops.plan.storage.database.transactions.init.OperationCriticalTransaction;
@ -58,7 +58,7 @@ public class DBAccessLock {
synchronized (lockObject) {
lockObject.wait();
if (database.getState() == Database.State.CLOSED) {
throw new EnableException("Database failed to open, Query has failed. (This exception is necessary to not keep query threads waiting)");
throw new DBClosedException("Database failed to open, Query has failed. (This exception is necessary to not keep query threads waiting)");
}
}
}

View File

@ -19,13 +19,21 @@ package com.djrapitops.plan.storage.database;
import com.djrapitops.plan.SubSystem;
import com.djrapitops.plan.exceptions.EnableException;
import com.djrapitops.plan.exceptions.database.DBInitException;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.config.paths.DatabaseSettings;
import com.djrapitops.plan.settings.locale.Locale;
import com.djrapitops.plan.settings.locale.lang.PluginLang;
import net.playeranalytics.plugin.server.PluginLogger;
import org.jetbrains.annotations.NotNull;
import javax.inject.Singleton;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Paths;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Stream;
/**
* System that holds the active databases.
@ -35,6 +43,7 @@ import java.util.Set;
@Singleton
public class DBSystem implements SubSystem {
protected final PlanConfig config;
protected final Locale locale;
private final SQLiteDB.Factory sqLiteFactory;
protected final PluginLogger logger;
@ -43,10 +52,12 @@ public class DBSystem implements SubSystem {
protected final Set<Database> databases;
public DBSystem(
PlanConfig config,
Locale locale,
SQLiteDB.Factory sqLiteDB,
PluginLogger logger
) {
this.config = config;
this.locale = locale;
this.sqLiteFactory = sqLiteDB;
this.logger = logger;
@ -94,7 +105,30 @@ public class DBSystem implements SubSystem {
} catch (DBInitException e) {
Throwable cause = e.getCause();
String message = cause == null ? e.getMessage() : cause.getMessage();
throw new EnableException(db.getType().getName() + " init failure: " + message, cause);
if (message.contains("The driver has not received any packets from the server.")) {
throw new EnableException(getMySQLConnectionFailureMessage());
} else {
throw new EnableException("Failed to start " + db.getType().getName() + ": " + message, cause);
}
}
}
@NotNull
private String getMySQLConnectionFailureMessage() {
return "Failed to start " + db.getType().getName() + ": Communications link failure. Plan could not connect to MySQL-" +
"\n- Check that database address '" + config.get(DatabaseSettings.MYSQL_HOST) + ":" + config.get(DatabaseSettings.MYSQL_PORT) + "' accessible." +
"\n- Check that database called '" + config.get(DatabaseSettings.MYSQL_DATABASE) + "' exists inside MySQL." +
"\n- Check that MySQL user '" + config.get(DatabaseSettings.MYSQL_USER) + "' has privileges to access the database." +
"\n- Check that other MySQL settings in Plan config correct." +
(isInsideDocker() ? "\n- Check that your docker container networking is set up correctly https://pterodactyl.io/tutorials/mysql_setup.html (Since your server is running inside a docker)" : "") +
"\n More help: https://github.com/plan-player-analytics/Plan/wiki/Bungee-Set-Up#step-2-create-a-mysql-database-for-plan";
}
private boolean isInsideDocker() {
try (Stream<String> stream = Files.lines(Paths.get("/proc/1/cgroup"))) {
return stream.anyMatch(line -> line.contains("/docker"));
} catch (IOException | InvalidPathException | SecurityException e) {
return false;
}
}

View File

@ -16,6 +16,7 @@
*/
package com.djrapitops.plan.storage.database;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.locale.Locale;
import net.playeranalytics.plugin.server.PluginLogger;
@ -32,12 +33,13 @@ public class ProxyDBSystem extends DBSystem {
@Inject
public ProxyDBSystem(
PlanConfig config,
Locale locale,
MySQLDB mySQLDB,
SQLiteDB.Factory sqLiteDB,
PluginLogger logger
) {
super(locale, sqLiteDB, logger);
super(config, locale, sqLiteDB, logger);
databases.add(mySQLDB);
db = mySQLDB;
}

View File

@ -19,6 +19,7 @@ package com.djrapitops.plan.utilities.logging;
import com.djrapitops.plan.PlanPlugin;
import com.djrapitops.plan.delivery.formatting.Formatters;
import com.djrapitops.plan.exceptions.ExceptionWithContext;
import com.djrapitops.plan.exceptions.database.DBClosedException;
import com.djrapitops.plan.identification.properties.ServerProperties;
import com.djrapitops.plan.storage.file.PlanFiles;
import com.djrapitops.plan.utilities.java.Lists;
@ -92,6 +93,10 @@ public class PluginErrorLogger implements ErrorLogger {
}
private void log(Consumer<String> logMethod, Throwable throwable, ErrorContext context) {
if (isExceptionThatShouldNotBeLogged(throwable)) {
return;
}
String errorName = throwable.getClass().getSimpleName();
String hash = hash(throwable);
Path logsDir = files.getLogsDirectory();
@ -105,6 +110,10 @@ public class PluginErrorLogger implements ErrorLogger {
}
}
private boolean isExceptionThatShouldNotBeLogged(Throwable throwable) {
return throwable instanceof DBClosedException || (throwable.getCause() != null && isExceptionThatShouldNotBeLogged(throwable.getCause()));
}
private void logToFile(Path errorLog, Throwable throwable, ErrorContext context, String hash) {
if (Files.exists(errorLog)) {
logExisting(errorLog, throwable, context, hash);

View File

@ -16,7 +16,6 @@
*/
package utilities.dagger;
import com.djrapitops.plan.exceptions.EnableException;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.config.paths.DatabaseSettings;
import com.djrapitops.plan.settings.locale.Locale;
@ -40,9 +39,9 @@ public class DBSystemModule {
MySQLDB mySQLDB,
PluginLogger logger
) {
return new DBSystem(locale, sqLiteDB, logger) {
return new DBSystem(config, locale, sqLiteDB, logger) {
@Override
public void enable() throws EnableException {
public void enable() {
databases.add(sqLiteDB.usingDefaultFile());
databases.add(mySQLDB);
String dbType = config.get(DatabaseSettings.TYPE).toLowerCase().trim();

View File

@ -36,8 +36,6 @@ import javax.inject.Singleton;
@Singleton
public class FabricDBSystem extends DBSystem {
private final PlanConfig config;
@Inject
public FabricDBSystem(
Locale locale,
@ -46,8 +44,7 @@ public class FabricDBSystem extends DBSystem {
PlanConfig config,
PluginLogger logger
) {
super(locale, sqLiteDB, logger);
this.config = config;
super(config, locale, sqLiteDB, logger);
databases.add(mySQLDB);
databases.add(sqLiteDB.usingDefaultFile());

View File

@ -32,8 +32,6 @@ import javax.inject.Singleton;
@Singleton
public class NukkitDBSystem extends DBSystem {
private final PlanConfig config;
@Inject
public NukkitDBSystem(
Locale locale,
@ -42,8 +40,7 @@ public class NukkitDBSystem extends DBSystem {
PlanConfig config,
PluginLogger logger
) {
super(locale, sqLiteDB, logger);
this.config = config;
super(config, locale, sqLiteDB, logger);
databases.add(mySQLDB);
databases.add(sqLiteDB.usingDefaultFile());

View File

@ -32,7 +32,6 @@ import javax.inject.Singleton;
@Singleton
public class SpongeDBSystem extends DBSystem {
private final PlanConfig config;
@Inject
public SpongeDBSystem(
@ -42,8 +41,7 @@ public class SpongeDBSystem extends DBSystem {
PlanConfig config,
PluginLogger logger
) {
super(locale, sqLiteDB, logger);
this.config = config;
super(config, locale, sqLiteDB, logger);
databases.add(mySQLDB);
databases.add(sqLiteDB.usingDefaultFile());