feat(error-handling): add subcategories to fatal errors and display corresponding error messages

mrts · mrts · commit f473ddfede78 · 2021-06-12T00:39:11.000+03:00
diff --git a/lib/libelectronic-id b/lib/libelectronic-id
@@ -1 +1 @@
-Subproject commit f923624bbedebbdd1cd5aacba1fa5f9cdaa34cd1
+Subproject commit 5515af4b9f7b556528156476f24adabf311929b0
diff --git a/src/app/getcommandhandler.cpp b/src/app/getcommandhandler.cpp
@@ -42,7 +42,7 @@ CommandHandler::ptr getCommandHandler(const CommandWithArguments& cmd)
     case CommandType::SIGN:
         return std::make_unique<Sign>(cmdCopy);
     default:
-        throw std::invalid_argument("Unknown command '" + std::string(cmdType)
-                                    + "' in getCommandHandler()");
+        throw ArgumentError("Unknown command '" + std::string(cmdType)
+                            + "' in getCommandHandler()");
     }
 }
diff --git a/src/controller/application.hpp b/src/controller/application.hpp
@@ -26,12 +26,6 @@
 
 #include "commands.hpp"
 
-class ArgumentError : public std::runtime_error
-{
-public:
-    using std::runtime_error::runtime_error;
-};
-
 class Application final : public QApplication
 {
     Q_OBJECT
diff --git a/src/controller/commands.cpp b/src/controller/commands.cpp
@@ -45,7 +45,7 @@ CommandType commandNameToCommandType(const QString& cmdName)
     try {
         return SUPPORTED_COMMANDS.at(cmdName);
     } catch (const std::out_of_range&) {
-        throw std::invalid_argument("Command '" + cmdName.toStdString() + "' is not supported");
+        throw ArgumentError("Command '" + cmdName.toStdString() + "' is not supported");
     }
 }
 
diff --git a/src/controller/commands.hpp b/src/controller/commands.hpp
@@ -60,3 +60,9 @@ CommandType commandNameToCommandType(const QString& cmdName);
 
 using CommandWithArguments = std::pair<CommandType, QVariantMap>;
 using CommandWithArgumentsPtr = std::unique_ptr<CommandWithArguments>;
+
+class ArgumentError : public std::runtime_error
+{
+public:
+    using std::runtime_error::runtime_error;
+};
diff --git a/src/controller/controller.cpp b/src/controller/controller.cpp
@@ -44,8 +44,7 @@ using namespace electronic_id;
 namespace
 {
 
-// TODO: Should we use more detailed error codes? E.g. report input data error back to the website
-// etc.
+const QString RESP_INVALID_ARGUMENT = QStringLiteral("ERR_WEBEID_INVALID_ARGUMENT");
 const QString RESP_TECH_ERROR = QStringLiteral("ERR_WEBEID_NATIVE_FATAL");
 const QString RESP_USER_CANCEL = QStringLiteral("ERR_WEBEID_USER_CANCELLED");
 
@@ -99,7 +98,8 @@ void Controller::run()
         commandHandler = getCommandHandler(*command);
 
         startCommandExecution();
-
+    } catch (const ArgumentError& error) {
+        onCriticalFailureImpl(error.what(), FatalErrorType::ARGUMENT_ERROR);
     } catch (const std::exception& error) {
         onCriticalFailure(error.what());
     }
@@ -328,15 +328,22 @@ void Controller::onDialogCancel()
 }
 
 void Controller::onCriticalFailure(const QString& error)
+{
+    onCriticalFailureImpl(error, FatalErrorType::OTHER_ERROR);
+}
+
+void Controller::onCriticalFailureImpl(const QString& error, const FatalErrorType errorType)
 {
     qCritical() << "Exiting due to command" << std::string(commandType())
                 << "fatal error:" << error;
-    writeResponseToStdOut(isInStdinMode, makeErrorObject(RESP_TECH_ERROR, error), commandType());
+    auto errorCode =
+        errorType == FatalErrorType::ARGUMENT_ERROR ? RESP_INVALID_ARGUMENT : RESP_TECH_ERROR;
+    writeResponseToStdOut(isInStdinMode, makeErrorObject(errorCode, error), commandType());
 
     // Dispose the UI.
     window.reset();
 
-    WebEidUI::showFatalError();
+    WebEidUI::showFatalError(errorType);
 
     exit();
 }
diff --git a/src/controller/controller.hpp b/src/controller/controller.hpp
@@ -87,6 +87,8 @@ class Controller : public QObject
     void connectRetry(const ControllerChildThread* childThread);
     void saveChildThreadPtrAndConnectFailureFinish(ControllerChildThread* childThread);
     void stopCardEventMonitorThread();
+    void onCriticalFailureImpl(const QString& error,
+                               const FatalErrorType errorType = FatalErrorType::OTHER_ERROR);
     void exit();
     void waitForChildThreads();
     CommandType commandType();
diff --git a/src/controller/inputoutputmode.cpp b/src/controller/inputoutputmode.cpp
@@ -22,6 +22,9 @@
 
 #include "inputoutputmode.hpp"
 
+#include "pcsc-cpp/pcsc-cpp.hpp"
+#include "electronic-id/electronic-id.hpp"
+
 #include <QJsonDocument>
 #include <QJsonObject>
 
@@ -48,6 +51,7 @@ void setIoStreamsToBinaryMode()
 #endif
 
 using namespace pcsc_cpp;
+using namespace electronic_id;
 
 uint32_t readMessageLength(std::istream& input)
 {
@@ -71,15 +75,13 @@ CommandWithArgumentsPtr readCommandFromStdin()
     const auto messageLength = readMessageLength(std::cin);
 
     if (messageLength < 5) {
-        // FIXME: Pass errors back up to caller in stdin mode.
-        throw std::invalid_argument("readCommandFromStdin: Message length is "
-                                    + std::to_string(messageLength) + ", at least 5 required");
+        throw ArgumentError("readCommandFromStdin: Message length is "
+                            + std::to_string(messageLength) + ", at least 5 required");
     }
 
     if (messageLength > 8192) {
-        throw std::invalid_argument("readCommandFromStdin: Message length "
-                                    + std::to_string(messageLength)
-                                    + " exceeds maximum allowed length 8192");
+        throw ArgumentError("readCommandFromStdin: Message length " + std::to_string(messageLength)
+                            + " exceeds maximum allowed length 8192");
     }
 
     auto message = QByteArray(int(messageLength), '\0');
@@ -88,18 +90,16 @@ CommandWithArgumentsPtr readCommandFromStdin()
     const auto json = QJsonDocument::fromJson(message);
 
     if (!json.isObject()) {
-        // FIXME: Pass errors back up to caller in stdin mode.
-        throw std::invalid_argument("readCommandFromStdin: Invalid JSON, not an object");
+        throw ArgumentError("readCommandFromStdin: Invalid JSON, not an object");
     }
 
     const auto jsonObject = json.object();
     const auto command = jsonObject["command"];
     const auto arguments = jsonObject["arguments"];
 
     if (!command.isString() || !arguments.isObject()) {
-        // FIXME: Pass errors back up to caller in stdin mode.
-        throw std::invalid_argument("readCommandFromStdin: Invalid JSON, the main object does not "
-                                    "contain a 'command' string and 'arguments' object");
+        throw ArgumentError("readCommandFromStdin: Invalid JSON, the main object does not "
+                            "contain a 'command' string and 'arguments' object");
     }
 
     return std::make_unique<CommandWithArguments>(commandNameToCommandType(command.toString()),
diff --git a/src/controller/inputoutputmode.hpp b/src/controller/inputoutputmode.hpp
@@ -24,8 +24,6 @@
 
 #include "commands.hpp"
 
-#include "pcsc-cpp/pcsc-cpp.hpp"
-
 CommandWithArgumentsPtr readCommandFromStdin();
 
 void writeResponseLength(std::ostream& stream, const uint32_t responseLength);
diff --git a/src/controller/retriableerror.hpp b/src/controller/retriableerror.hpp
@@ -28,6 +28,9 @@
 #include <QMetaType>
 #include <QDebug>
 
+// TODO: rename this file and .cpp to errors.{c,h}pp
+enum class FatalErrorType { PROGRAMMING_ERROR, ARGUMENT_ERROR, IO_ERROR, OTHER_ERROR };
+
 enum class RetriableError {
     // libpcsc-cpp
     SMART_CARD_SERVICE_IS_NOT_RUNNING,
diff --git a/src/controller/ui.hpp b/src/controller/ui.hpp
@@ -43,7 +43,7 @@ class WebEidUI : public QDialog
     // Factory function that creates and shows the dialog that implements this interface.
     static ptr createAndShowDialog(const CommandType command);
 
-    static void showFatalError();
+    static void showFatalError(const FatalErrorType errorType);
 
     virtual void showWaitingForCardPage(const CommandType commandType) = 0;
 
diff --git a/src/controller/utils.hpp b/src/controller/utils.hpp
@@ -23,19 +23,20 @@
 #pragma once
 
 #include "pcsc-cpp/pcsc-cpp-utils.hpp"
+#include "electronic-id/electronic-id.hpp"
 
 #define REQUIRE_NON_NULL(val)                                                                      \
     if (!val) {                                                                                    \
-        throw std::logic_error("Null " + std::string(#val) + " in "                                \
-                               + pcsc_cpp::removeAbsolutePathPrefix(__FILE__) + ':'                \
-                               + std::to_string(__LINE__) + ':' + __func__);                       \
+        throw electronic_id::ProgrammingError("Null " + std::string(#val) + " in "                 \
+                                              + pcsc_cpp::removeAbsolutePathPrefix(__FILE__) + ':' \
+                                              + std::to_string(__LINE__) + ':' + __func__);        \
     }
 
 #define REQUIRE_NOT_EMPTY_CONTAINS_NON_NULL_PTRS(vec)                                              \
     if (vec.empty()) {                                                                             \
-        throw std::logic_error(std::string(#vec) + " is empty in "                                 \
-                               + pcsc_cpp::removeAbsolutePathPrefix(__FILE__) + ':'                \
-                               + std::to_string(__LINE__) + ':' + __func__);                       \
+        throw electronic_id::ProgrammingError(std::string(#vec) + " is empty in "                  \
+                                              + pcsc_cpp::removeAbsolutePathPrefix(__FILE__) + ':' \
+                                              + std::to_string(__LINE__) + ':' + __func__);        \
     }                                                                                              \
     for (const auto& ptr : vec) {                                                                  \
         REQUIRE_NON_NULL(ptr)                                                                      \
diff --git a/src/controller/writeresponse.cpp b/src/controller/writeresponse.cpp
@@ -24,6 +24,8 @@
 #include "inputoutputmode.hpp"
 #include "logging.hpp"
 
+#include "electronic-id/electronic-id.hpp"
+
 #include <QJsonDocument>
 #include <QJsonObject>
 
@@ -34,8 +36,8 @@ QByteArray resultToJson(const QVariantMap& result, const std::string& commandTyp
 {
     const auto json = QJsonDocument::fromVariant(result);
     if (!json.isObject()) {
-        throw std::logic_error("Controller::resultToJson: command " + commandType
-                               + " did not return a JSON object");
+        throw electronic_id::ProgrammingError("Controller::resultToJson: command " + commandType
+                                              + " did not return a JSON object");
     }
 
     return json.toJson(QJsonDocument::Compact);
diff --git a/src/ui/ui.cpp b/src/ui/ui.cpp
@@ -33,8 +33,8 @@ WebEidUI::ptr WebEidUI::createAndShowDialog(const CommandType command)
     return dialog;
 }
 
-void WebEidUI::showFatalError()
+void WebEidUI::showFatalError(const FatalErrorType errorType)
 {
     auto dialog = WebEidDialog {};
-    dialog.showFatalErrorPage();
+    dialog.showFatalErrorPage(errorType);
 }
diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp
@@ -115,10 +115,14 @@ WebEidDialog::~WebEidDialog()
     delete ui;
 }
 
-void WebEidDialog::showFatalErrorPage()
+void WebEidDialog::showFatalErrorPage(const FatalErrorType errorType)
 {
     ui->messagePageTitleLabel->setText(tr("Fatal error"));
     ui->fatalError->show();
+    // TODO: use switch/case to cover other cases.
+    if (errorType == FatalErrorType::ARGUMENT_ERROR) {
+        ui->fatalErrorLabel->setText(tr("Invalid argument was passed to the application"));
+    }
     ui->connectCardLabel->hide();
     ui->cardChipIcon->hide();
     ui->helpButton->show();
@@ -295,7 +299,6 @@ void WebEidDialog::onVerifyPinFailed(const electronic_id::VerifyPinFailed::Statu
 
     QString message;
 
-    // FIXME: don't allow retry in case of UNKNOWN_ERROR
     switch (status) {
     case Status::RETRY_ALLOWED:
         message = tr("Incorrect PIN, %n retries left", nullptr, retriesLeft);
@@ -316,9 +319,6 @@ void WebEidDialog::onVerifyPinFailed(const electronic_id::VerifyPinFailed::Statu
     case Status::PIN_ENTRY_CANCEL:
         message = tr("PIN pad PIN entry cancelled");
         break;
-    case Status::UNKNOWN_ERROR:
-        message = tr("Technical error");
-        break;
     }
 
     ui->pinErrorLabel->setHidden(message.isEmpty());
diff --git a/src/ui/webeiddialog.hpp b/src/ui/webeiddialog.hpp
@@ -51,7 +51,7 @@ class WebEidDialog : public WebEidUI
     void showWaitingForCardPage(const CommandType commandType) override;
     QString getPin() override;
 
-    void showFatalErrorPage();
+    void showFatalErrorPage(const FatalErrorType errorType);
 
 public: // slots
     void onSmartCardStatusUpdate(const RetriableError status) override;
diff --git a/tests/mock-ui/mock-ui.cpp b/tests/mock-ui/mock-ui.cpp
@@ -28,4 +28,4 @@ WebEidUI::ptr WebEidUI::createAndShowDialog(const CommandType)
     return std::make_unique<MockUI>();
 }
 
-void WebEidUI::showFatalError() {}
+void WebEidUI::showFatalError(const FatalErrorType) {}

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ CommandHandler::ptr getCommandHandler(const CommandWithArguments& cmd)`
`42`	`42`	`case CommandType::SIGN:`
`43`	`43`	`return std::make_unique<Sign>(cmdCopy);`
`44`	`44`	`default:`
`45`		`- throw std::invalid_argument("Unknown command '" + std::string(cmdType)`
`46`		`- + "' in getCommandHandler()");`
	`45`	`+ throw ArgumentError("Unknown command '" + std::string(cmdType)`
	`46`	`+ + "' in getCommandHandler()");`
`47`	`47`	`}`
`48`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ CommandType commandNameToCommandType(const QString& cmdName)`
`45`	`45`	`try {`
`46`	`46`	`return SUPPORTED_COMMANDS.at(cmdName);`
`47`	`47`	`} catch (const std::out_of_range&) {`
`48`		`- throw std::invalid_argument("Command '" + cmdName.toStdString() + "' is not supported");`
	`48`	`+ throw ArgumentError("Command '" + cmdName.toStdString() + "' is not supported");`
`49`	`49`	`}`
`50`	`50`	`}`
`51`	`51`