Skip to content

Commit

Permalink
feat(error-handling): add subcategories to fatal errors and display c…
Browse files Browse the repository at this point in the history
…orresponding error messages
  • Loading branch information
mrts committed Jun 11, 2021
1 parent f3f20be commit f473ddf
Show file tree
Hide file tree
Showing 17 changed files with 59 additions and 46 deletions.
4 changes: 2 additions & 2 deletions src/app/getcommandhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()");
}
}
6 changes: 0 additions & 6 deletions src/controller/application.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/controller/commands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/controller/commands.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
17 changes: 12 additions & 5 deletions src/controller/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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();
}
Expand Down
2 changes: 2 additions & 0 deletions src/controller/controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
22 changes: 11 additions & 11 deletions src/controller/inputoutputmode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@

#include "inputoutputmode.hpp"

#include "pcsc-cpp/pcsc-cpp.hpp"
#include "electronic-id/electronic-id.hpp"

#include <QJsonDocument>
#include <QJsonObject>

Expand All @@ -48,6 +51,7 @@ void setIoStreamsToBinaryMode()
#endif

using namespace pcsc_cpp;
using namespace electronic_id;

uint32_t readMessageLength(std::istream& input)
{
Expand All @@ -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');
Expand All @@ -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()),
Expand Down
2 changes: 0 additions & 2 deletions src/controller/inputoutputmode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@

#include "commands.hpp"

#include "pcsc-cpp/pcsc-cpp.hpp"

CommandWithArgumentsPtr readCommandFromStdin();

void writeResponseLength(std::ostream& stream, const uint32_t responseLength);
3 changes: 3 additions & 0 deletions src/controller/retriableerror.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/controller/ui.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
13 changes: 7 additions & 6 deletions src/controller/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
6 changes: 4 additions & 2 deletions src/controller/writeresponse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include "inputoutputmode.hpp"
#include "logging.hpp"

#include "electronic-id/electronic-id.hpp"

#include <QJsonDocument>
#include <QJsonObject>

Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/ui/ui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
10 changes: 5 additions & 5 deletions src/ui/webeiddialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion src/ui/webeiddialog.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/mock-ui/mock-ui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ WebEidUI::ptr WebEidUI::createAndShowDialog(const CommandType)
return std::make_unique<MockUI>();
}

void WebEidUI::showFatalError() {}
void WebEidUI::showFatalError(const FatalErrorType) {}

0 comments on commit f473ddf

Please sign in to comment.