Improve reporting of version file errors.x

9 files changed, 154 insertions, 102 deletions

diff --git a/logic/MMCJson.h b/logic/MMCJson.h
index b0d898fc..f2cc4b31 100644
--- a/logic/MMCJson.h
+++ b/logic/MMCJson.h
@@ -15,10 +15,6 @@ class JSONValidationError : public MMCError
 {
 public:
 	JSONValidationError(QString cause) : MMCError(cause) {};
-	virtual QString errorName()
-	{
-		return "JSONValidationError";
-	};
 	virtual ~JSONValidationError() {};
 };
 
diff --git a/logic/OneSixFTBInstance.cpp b/logic/OneSixFTBInstance.cpp
index cdb3f53e..8f70ed08 100644
--- a/logic/OneSixFTBInstance.cpp
+++ b/logic/OneSixFTBInstance.cpp
@@ -17,7 +17,14 @@ OneSixFTBInstance::OneSixFTBInstance(const QString &rootDir, SettingsObject *set
 
 void OneSixFTBInstance::init()
 {
-	reloadVersion();
+	try
+	{
+		reloadVersion();
+	}
+	catch(MMCError & e)
+	{
+		// QLOG_ERROR() << "Caught exception on instance init: " << e.cause();
+	}
 }
 
 void OneSixFTBInstance::copy(const QDir &newDir)
diff --git a/logic/OneSixInstance.cpp b/logic/OneSixInstance.cpp
index f6fe49f1..3c1f6545 100644
--- a/logic/OneSixInstance.cpp
+++ b/logic/OneSixInstance.cpp
@@ -41,9 +41,17 @@ OneSixInstance::OneSixInstance(const QString &rootDir, SettingsObject *settings,
 
 void OneSixInstance::init()
 {
+	// FIXME: why is this decided here? what does this even mean?
 	if (QDir(instanceRoot()).exists("version.json"))
 	{
-		reloadVersion();
+		try
+		{
+			reloadVersion();
+		}
+		catch(MMCError & e)
+		{
+			// QLOG_ERROR() << "Caught exception on instance init: " << e.cause();
+		}
 	}
 	else
 	{
@@ -317,7 +325,7 @@ QString OneSixInstance::currentVersionId() const
 	return intendedVersionId();
 }
 
-bool OneSixInstance::reloadVersion()
+void OneSixInstance::reloadVersion()
 {
 	I_D(OneSixInstance);
 
@@ -327,16 +335,15 @@ bool OneSixInstance::reloadVersion()
 		d->vanillaVersion->reload(true, externalPatches());
 		setFlags(flags() & ~VersionBrokenFlag);
 		emit versionReloaded();
-		return true;
 	}
-	catch(MMCError error)
+	catch(MMCError & error)
 	{
 		d->version->clear();
 		d->vanillaVersion->clear();
 		setFlags(flags() | VersionBrokenFlag);
 		//TODO: rethrow to show some error message(s)?
 		emit versionReloaded();
-		return false;
+		throw;
 	}
 }
 
diff --git a/logic/OneSixInstance.h b/logic/OneSixInstance.h
index c7ef2ee8..d2bc9b01 100644
--- a/logic/OneSixInstance.h
+++ b/logic/OneSixInstance.h
@@ -53,8 +53,12 @@ public:
 
 	virtual QDialog *createModEditDialog(QWidget *parent) override;
 
-	/// reload the full version json files. return true on success!
-	bool reloadVersion();
+	/**
+	 * reload the full version json files. return true on success!
+	 * 
+	 * throws various exceptions :3
+	 */
+	void reloadVersion();
 	/// clears all version information in preparation for an update
 	void clearVersion();
 	/// get the current full version info
diff --git a/logic/OneSixUpdate.cpp b/logic/OneSixUpdate.cpp
index 750aeabb..65f30cda 100644
--- a/logic/OneSixUpdate.cpp
+++ b/logic/OneSixUpdate.cpp
@@ -48,7 +48,7 @@ void OneSixUpdate::executeTask()
 	QDir mcDir(m_inst->minecraftRoot());
 	if (!mcDir.exists() && !mcDir.mkpath("."))
 	{
-		emitFailed("Failed to create bin folder.");
+		emitFailed(tr("Failed to create folder for minecraft binaries."));
 		return;
 	}
 
@@ -60,7 +60,7 @@ void OneSixUpdate::executeTask()
 		if (targetVersion == nullptr)
 		{
 			// don't do anything if it was invalid
-			emitFailed("The specified Minecraft version is invalid. Choose a different one.");
+			emitFailed(tr("The specified Minecraft version is invalid. Choose a different one."));
 			return;
 		}
 		versionFileStart();
@@ -108,20 +108,19 @@ void OneSixUpdate::versionFileFinished()
 		QSaveFile vfile1(version1);
 		if (!vfile1.open(QIODevice::Truncate | QIODevice::WriteOnly))
 		{
-			emitFailed("Can't open " + version1 + " for writing.");
+			emitFailed(tr("Can't open %1 for writing.").arg(version1));
 			return;
 		}
 		auto data = std::dynamic_pointer_cast<ByteArrayDownload>(DlJob)->m_data;
 		qint64 actual = 0;
 		if ((actual = vfile1.write(data)) != data.size())
 		{
-			emitFailed("Failed to write into " + version1 + ". Written " + actual + " out of " +
-					   data.size() + '.');
+			emitFailed(tr("Failed to write into %1. Written %2 out of %3.").arg(version1).arg(actual).arg(data.size()));
 			return;
 		}
 		if (!vfile1.commit())
 		{
-			emitFailed("Can't commit changes to " + version1);
+			emitFailed(tr("Can't commit changes to %1").arg(version1));
 			return;
 		}
 	}
@@ -136,14 +135,13 @@ void OneSixUpdate::versionFileFinished()
 	{
 		finfo.remove();
 	}
-	inst->reloadVersion();
-
+	// NOTE: Version is reloaded in jarlibStart
 	jarlibStart();
 }
 
 void OneSixUpdate::versionFileFailed()
 {
-	emitFailed("Failed to download the version description. Try again.");
+	emitFailed(tr("Failed to download the version description. Try again."));
 }
 
 void OneSixUpdate::assetIndexStart()
@@ -180,7 +178,7 @@ void OneSixUpdate::assetIndexFinished()
 	QString asset_fname = "assets/indexes/" + assetName + ".json";
 	if (!AssetsUtils::loadAssetsIndexJson(asset_fname, &index))
 	{
-		emitFailed("Failed to read the assets index!");
+		emitFailed(tr("Failed to read the assets index!"));
 	}
 
 	QList<Md5EtagDownloadPtr> dls;
@@ -216,7 +214,7 @@ void OneSixUpdate::assetIndexFinished()
 
 void OneSixUpdate::assetIndexFailed()
 {
-	emitFailed("Failed to download the assets index!");
+	emitFailed(tr("Failed to download the assets index!"));
 }
 
 void OneSixUpdate::assetsFinished()
@@ -226,7 +224,7 @@ void OneSixUpdate::assetsFinished()
 
 void OneSixUpdate::assetsFailed()
 {
-	emitFailed("Failed to download assets!");
+	emitFailed(tr("Failed to download assets!"));
 }
 
 void OneSixUpdate::jarlibStart()
@@ -234,11 +232,18 @@ void OneSixUpdate::jarlibStart()
 	setStatus(tr("Getting the library files from Mojang..."));
 	QLOG_INFO() << m_inst->name() << ": downloading libraries";
 	OneSixInstance *inst = (OneSixInstance *)m_inst;
-	bool successful = inst->reloadVersion();
-	if (!successful)
+	try
+	{
+		inst->reloadVersion();
+	}
+	catch(MMCError & e)
+	{
+		emitFailed(e.cause());
+		return;
+	}
+	catch(...)
 	{
-		emitFailed("Failed to load the version description file. It might be "
-				   "corrupted, missing or simply too new.");
+		emitFailed(tr("Failed to load the version description file for reasons unknown."));
 		return;
 	}
 
@@ -326,6 +331,5 @@ void OneSixUpdate::jarlibFailed()
 {
 	QStringList failed = jarlibDownloadJob->getFailedFiles();
 	QString failed_all = failed.join("\n");
-	emitFailed("Failed to download the following files:\n" + failed_all +
-			   "\n\nPlease try again.");
+	emitFailed(tr("Failed to download the following files:\n%1\n\nPlease try again.").arg(failed_all));
 }
diff --git a/logic/OneSixVersionBuilder.cpp b/logic/OneSixVersionBuilder.cpp
index 8abeb0d8..8eacbce4 100644
--- a/logic/OneSixVersionBuilder.cpp
+++ b/logic/OneSixVersionBuilder.cpp
@@ -38,7 +38,8 @@ OneSixVersionBuilder::OneSixVersionBuilder()
 {
 }
 
-void OneSixVersionBuilder::build(VersionFinal *version, OneSixInstance *instance, const bool onlyVanilla, const QStringList &external)
+void OneSixVersionBuilder::build(VersionFinal *version, OneSixInstance *instance,
+								 const bool onlyVanilla, const QStringList &external)
 {
 	OneSixVersionBuilder builder;
 	builder.m_version = version;
@@ -46,7 +47,8 @@ void OneSixVersionBuilder::build(VersionFinal *version, OneSixInstance *instance
 	builder.buildInternal(onlyVanilla, external);
 }
 
-void OneSixVersionBuilder::readJsonAndApplyToVersion(VersionFinal *version, const QJsonObject &obj)
+void OneSixVersionBuilder::readJsonAndApplyToVersion(VersionFinal *version,
+													 const QJsonObject &obj)
 {
 	OneSixVersionBuilder builder;
 	builder.m_version = version;
@@ -62,17 +64,19 @@ void OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringLi
 	QDir patches(root.absoluteFilePath("patches/"));
 
 	// if we do external files, do just those.
-	if(!external.isEmpty()) for (auto fileName : external)
-	{
-		QLOG_INFO() << "Reading" << fileName;
-		auto file = parseJsonFile(QFileInfo(fileName), false, fileName.endsWith("pack.json"));
-		file->name = QFileInfo(fileName).fileName();
-		file->fileId = "org.multimc.external." + file->name;
-		file->version = QString();
-		file->mcVersion = QString();
-		file->applyTo(m_version);
-		m_version->versionFiles.append(file);
-	}
+	if (!external.isEmpty())
+		for (auto fileName : external)
+		{
+			QLOG_INFO() << "Reading" << fileName;
+			auto file =
+				parseJsonFile(QFileInfo(fileName), false, fileName.endsWith("pack.json"));
+			file->name = QFileInfo(fileName).fileName();
+			file->fileId = "org.multimc.external." + file->name;
+			file->version = QString();
+			file->mcVersion = QString();
+			file->applyTo(m_version);
+			m_version->versionFiles.append(file);
+		}
 	// else, if there's custom json, we just do that.
 	else if (QFile::exists(root.absoluteFilePath("custom.json")))
 	{
@@ -84,48 +88,52 @@ void OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringLi
 		file->version = QString();
 		file->applyTo(m_version);
 		m_version->versionFiles.append(file);
-		// QObject::tr("The version descriptors of this instance are not compatible with the current version of MultiMC"));
+		// QObject::tr("The version descriptors of this instance are not compatible with the
+		// current version of MultiMC"));
 		// QObject::tr("Error while applying %1. Please check MultiMC-0.log for more info.")
 	}
 	// version.json -> patches/*.json -> user.json
-	else do
-	{
-		// version.json
-		QLOG_INFO() << "Reading version.json";
-		auto file = parseJsonFile(QFileInfo(root.absoluteFilePath("version.json")), false);
-		file->name = "Minecraft";
-		file->fileId = "org.multimc.version.json";
-		file->version = m_instance->intendedVersionId();
-		file->mcVersion = m_instance->intendedVersionId();
-		file->applyTo(m_version);
-		m_version->versionFiles.append(file);
-		// QObject::tr("Error while applying %1. Please check MultiMC-0.log for more info.").arg(root.absoluteFilePath("version.json")));
+	else
+		do
+		{
+			// version.json
+			QLOG_INFO() << "Reading version.json";
+			auto file = parseJsonFile(QFileInfo(root.absoluteFilePath("version.json")), false);
+			file->name = "Minecraft";
+			file->fileId = "org.multimc.version.json";
+			file->version = m_instance->intendedVersionId();
+			file->mcVersion = m_instance->intendedVersionId();
+			file->applyTo(m_version);
+			m_version->versionFiles.append(file);
+			// QObject::tr("Error while applying %1. Please check MultiMC-0.log for more
+			// info.").arg(root.absoluteFilePath("version.json")));
 
-		if (onlyVanilla)
-			break;
+			if (onlyVanilla)
+				break;
 
-		// patches/
-		// load all, put into map for ordering, apply in the right order
+			// patches/
+			// load all, put into map for ordering, apply in the right order
 
-		QMap<int, QPair<QString, VersionFilePtr>> files;
-		for (auto info : patches.entryInfoList(QStringList() << "*.json", QDir::Files))
-		{
-			QLOG_INFO() << "Reading" << info.fileName();
-			auto file = parseJsonFile(info, true);
-			if (files.contains(file->order))
+			QMap<int, QPair<QString, VersionFilePtr>> files;
+			for (auto info : patches.entryInfoList(QStringList() << "*.json", QDir::Files))
 			{
-				throw VersionBuildError(QObject::tr("%1 has the same order as %2").arg(file->fileId, files[file->order].second->fileId));
+				QLOG_INFO() << "Reading" << info.fileName();
+				auto file = parseJsonFile(info, true);
+				if (files.contains(file->order))
+				{
+					throw VersionBuildError(QObject::tr("%1 has the same order as %2").arg(
+						file->fileId, files[file->order].second->fileId));
+				}
+				files.insert(file->order, qMakePair(info.fileName(), file));
 			}
-			files.insert(file->order, qMakePair(info.fileName(), file));
-		}
-		for (auto order : files.keys())
-		{
-			QLOG_DEBUG() << "Applying file with order" << order;
-			auto & filePair = files[order];
-			filePair.second->applyTo(m_version);
-			m_version->versionFiles.append(filePair.second);
-		}
-	} while(0);
+			for (auto order : files.keys())
+			{
+				QLOG_DEBUG() << "Applying file with order" << order;
+				auto &filePair = files[order];
+				filePair.second->applyTo(m_version);
+				m_version->versionFiles.append(filePair.second);
+			}
+		} while (0);
 
 	// some final touches
 	finalizeVersion();
@@ -168,26 +176,30 @@ void OneSixVersionBuilder::readJsonAndApply(const QJsonObject &obj)
 	file->applyTo(m_version);
 	m_version->versionFiles.append(file);
 	// QObject::tr("Error while applying. Please check MultiMC-0.log for more info."));
-	// QObject::tr("The version descriptors of this instance are not compatible with the current version of MultiMC"));
+	// QObject::tr("The version descriptors of this instance are not compatible with the current
+	// version of MultiMC"));
 }
 
-VersionFilePtr OneSixVersionBuilder::parseJsonFile(const QFileInfo &fileInfo, const bool requireOrder, bool isFTB)
+VersionFilePtr OneSixVersionBuilder::parseJsonFile(const QFileInfo &fileInfo,
+												   const bool requireOrder, bool isFTB)
 {
 	QFile file(fileInfo.absoluteFilePath());
 	if (!file.open(QFile::ReadOnly))
 	{
-		throw JSONValidationError(QObject::tr("Unable to open %1: %2").arg(file.fileName(), file.errorString()));
+		throw JSONValidationError(QObject::tr("Unable to open the version file %1: %2.")
+									  .arg(fileInfo.fileName(), file.errorString()));
 	}
 	QJsonParseError error;
 	QJsonDocument doc = QJsonDocument::fromJson(file.readAll(), &error);
 	if (error.error != QJsonParseError::NoError)
 	{
-		throw JSONValidationError(QObject::tr("Unable to parse %1: %2 at %3")
-								  .arg(file.fileName(), error.errorString())
-								  .arg(error.offset));
+		throw JSONValidationError(QObject::tr("Unable to process the version file %1: %2 at %3.")
+									  .arg(fileInfo.fileName(), error.errorString())
+									  .arg(error.offset));
 	}
 	return VersionFile::fromJson(doc, file.fileName(), requireOrder, isFTB);
-	// QObject::tr("Error while reading %1. Please check MultiMC-0.log for more info.").arg(file.fileName());
+	// QObject::tr("Error while reading %1. Please check MultiMC-0.log for more
+	// info.").arg(file.fileName());
 }
 
 QMap<QString, int> OneSixVersionBuilder::readOverrideOrders(OneSixInstance *instance)
@@ -203,7 +215,7 @@ QMap<QString, int> OneSixVersionBuilder::readOverrideOrders(OneSixInstance *inst
 	if (!orderFile.open(QFile::ReadOnly))
 	{
 		QLOG_ERROR() << "Couldn't open" << orderFile.fileName()
-						<< " for reading:" << orderFile.errorString();
+					 << " for reading:" << orderFile.errorString();
 		QLOG_WARN() << "Ignoring overriden order";
 		return out;
 	}
@@ -211,9 +223,9 @@ QMap<QString, int> OneSixVersionBuilder::readOverrideOrders(OneSixInstance *inst
 	// and it's valid JSON
 	QJsonParseError error;
 	QJsonDocument doc = QJsonDocument::fromJson(orderFile.readAll(), &error);
-	if (error.error != QJsonParseError::NoError )
+	if (error.error != QJsonParseError::NoError)
 	{
-		QLOG_ERROR() << "Couldn't parse" << orderFile.fileName() << ":"	<< error.errorString();
+		QLOG_ERROR() << "Couldn't parse" << orderFile.fileName() << ":" << error.errorString();
 		QLOG_WARN() << "Ignoring overriden order";
 		return out;
 	}
@@ -231,7 +243,7 @@ QMap<QString, int> OneSixVersionBuilder::readOverrideOrders(OneSixInstance *inst
 			out.insert(it.key(), MMCJson::ensureInteger(it.value()));
 		}
 	}
-	catch (JSONValidationError err)
+	catch (JSONValidationError &err)
 	{
 		QLOG_ERROR() << "Couldn't parse" << orderFile.fileName() << ": bad file format";
 		QLOG_WARN() << "Ignoring overriden order";
@@ -262,4 +274,3 @@ bool OneSixVersionBuilder::writeOverrideOrders(const QMap<QString, int> &order,
 	orderFile.write(QJsonDocument(obj).toJson(QJsonDocument::Indented));
 	return true;
 }
-
diff --git a/logic/VersionFile.cpp b/logic/VersionFile.cpp
index 40dcb0c3..831b086e 100644
--- a/logic/VersionFile.cpp
+++ b/logic/VersionFile.cpp
@@ -311,9 +311,7 @@ void VersionFile::applyTo(VersionFinal *version)
 	{
 		if (minimumLauncherVersion > CURRENT_MINIMUM_LAUNCHER_VERSION)
 		{
-			throw VersionBuildError(
-				QString("%1 is for a different launcher version (%2), current supported is %3")
-					.arg(filename, minimumLauncherVersion, CURRENT_MINIMUM_LAUNCHER_VERSION));
+			throw LauncherVersionError(minimumLauncherVersion, CURRENT_MINIMUM_LAUNCHER_VERSION);
 		}
 	}
 
@@ -322,8 +320,7 @@ void VersionFile::applyTo(VersionFinal *version)
 		if (QRegExp(mcVersion, Qt::CaseInsensitive, QRegExp::Wildcard).indexIn(version->id) ==
 			-1)
 		{
-			throw VersionBuildError(
-				QString("%1 is for a different version of Minecraft").arg(filename));
+			throw MinecraftVersionMismatch(fileId, mcVersion, version->id);
 		}
 	}
 
@@ -471,7 +468,7 @@ void VersionFile::applyTo(VersionFinal *version)
 						(lib->dependType == RawLibrary::Hard && ourVersion != otherVersion))
 					{
 						throw VersionBuildError(
-							QString(
+							QObject::tr(
 								"Error resolving library dependencies between %1 and %2 in %3.")
 								.arg(otherLib->rawName(), lib->name, filename));
 					}
@@ -498,7 +495,7 @@ void VersionFile::applyTo(VersionFinal *version)
 						// it: fail
 						if (lib->dependType == RawLibrary::Hard)
 						{
-							throw VersionBuildError(QString(
+							throw VersionBuildError(QObject::tr(
 								"Error resolving library dependencies between %1 and %2 in %3.")
 														.arg(otherLib->rawName(), lib->name,
 															 filename));
diff --git a/logic/VersionFile.h b/logic/VersionFile.h
index 504fcbff..67d22b23 100644
--- a/logic/VersionFile.h
+++ b/logic/VersionFile.h
@@ -13,13 +13,39 @@ class VersionBuildError : public MMCError
 {
 public:
 	VersionBuildError(QString cause) : MMCError(cause) {};
-	virtual QString errorName()
-	{
-		return "VersionBuildError";
-	};
 	virtual ~VersionBuildError() {};
 };
 
+/**
+ * the base version file was meant for a newer version of the vanilla launcher than we support
+ */
+class LauncherVersionError : public VersionBuildError
+{
+public:
+	LauncherVersionError(int actual, int supported)
+		: VersionBuildError(QObject::tr(
+			  "The base version file of this instance was meant for a newer (%1) "
+			  "version of the vanilla launcher than this version of MultiMC supports (%2).")
+								.arg(actual)
+								.arg(supported)) {};
+	virtual ~LauncherVersionError() {};
+};
+
+/**
+ * some patch was intended for a different version of minecraft
+ */
+class MinecraftVersionMismatch : public VersionBuildError
+{
+public:
+	MinecraftVersionMismatch(QString fileId, QString mcVersion, QString parentMcVersion)
+		: VersionBuildError(QObject::tr("The patch %1 is for a different version of Minecraft "
+										"(%2) than that of the instance (%3).")
+								.arg(fileId)
+								.arg(mcVersion)
+								.arg(parentMcVersion)) {};
+	virtual ~MinecraftVersionMismatch() {};
+};
+
 struct RawLibrary;
 typedef std::shared_ptr<RawLibrary> RawLibraryPtr;
 struct RawLibrary
@@ -61,7 +87,7 @@ struct VersionFile
 {
 public: /* methods */
 	static VersionFilePtr fromJson(const QJsonDocument &doc, const QString &filename,
-								const bool requireOrder, const bool isFTB = false);
+								   const bool requireOrder, const bool isFTB = false);
 
 	static OneSixLibraryPtr createLibrary(RawLibraryPtr lib);
 	int findLibrary(QList<OneSixLibraryPtr> haystack, const QString &needle);
diff --git a/logic/VersionFinal.cpp b/logic/VersionFinal.cpp
index 48601d57..a057ecdd 100644
--- a/logic/VersionFinal.cpp
+++ b/logic/VersionFinal.cpp
@@ -112,7 +112,7 @@ std::shared_ptr<VersionFinal> VersionFinal::fromJson(const QJsonObject &obj)
 	{
 		OneSixVersionBuilder::readJsonAndApplyToVersion(version.get(), obj);
 	}
-	catch(MMCError err)
+	catch(MMCError & err)
 	{
 		return 0;
 	}