Skip to content

Commit 3be3311

Browse files
metsmametsma
committed
Make RetriableError separate class and avoid macros
Signed-off-by: Raul Metsma <[email protected]>
1 parent 9c5f9ff commit 3be3311

File tree

10 files changed

+132
-178
lines changed

10 files changed

+132
-178
lines changed

‎.github/workflows/cmake-linux-fedora.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ jobs:
1414
container: fedora:${{ matrix.container }}
1515
strategy:
1616
matrix:
17-
container: [41, 42, 43]
17+
container: [42, 43]
1818

1919
steps:
2020
- name: Install Deps
2121
run: dnf install -y --setopt=install_weak_deps=False git gcc-c++ cmake rpm-build openssl-devel pcsc-lite-devel qt6-qtsvg-devel qt6-qttools-devel gtest-devel
2222

23-
- uses: actions/checkout@v4
23+
- uses: actions/checkout@v5
2424
with:
2525
submodules: recursive
2626

‎src/controller/CMakeLists.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ add_library(controller STATIC
2222
logging.cpp
2323
logging.hpp
2424
qeid.hpp
25-
retriableerror.cpp
2625
retriableerror.hpp
2726
threads/cardeventmonitorthread.hpp
2827
threads/commandhandlerconfirmthread.hpp

‎src/controller/commands.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
#include "commands.hpp"
2424

25-
#include "magic_enum/magic_enum.hpp"
25+
#include <QMetaEnum>
2626

2727
#include <stdexcept>
2828
#include <map>
@@ -51,5 +51,5 @@ CommandType commandNameToCommandType(const QString& cmdName)
5151

5252
CommandType::operator std::string() const
5353
{
54-
return std::string(magic_enum::enum_name(value));
54+
return QMetaEnum::fromType<CommandType::CommandTypeEnum>().valueToKey(value);
5555
}

‎src/controller/commands.hpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
class CommandType
3131
{
32+
Q_GADGET
3233
public:
3334
enum CommandTypeEnum {
3435
INSERT_CARD,
@@ -39,20 +40,25 @@ class CommandType
3940
ABOUT,
4041
NONE = -1
4142
};
43+
Q_ENUM(CommandTypeEnum)
4244

43-
CommandType() = default;
44-
constexpr CommandType(const CommandTypeEnum _value) : value(_value) {}
45+
constexpr CommandType(CommandTypeEnum _value = NONE) noexcept : value(_value) {}
4546

46-
constexpr bool operator==(CommandTypeEnum other) const { return value == other; }
47-
constexpr bool operator!=(CommandTypeEnum other) const { return value != other; }
48-
constexpr operator CommandTypeEnum() const { return value; }
47+
constexpr bool operator==(CommandTypeEnum other) const noexcept { return value == other; }
48+
constexpr operator CommandTypeEnum() const noexcept { return value; }
4949

5050
operator std::string() const;
5151

5252
private:
53-
CommandTypeEnum value = NONE;
53+
CommandTypeEnum value;
5454
};
5555

56+
inline QDebug operator<<(QDebug dbg, const CommandType& ct)
57+
{
58+
QDebugStateSaver saver(dbg);
59+
return dbg.nospace() << CommandType::CommandTypeEnum(ct);
60+
}
61+
5662
extern const QString CMDLINE_GET_SIGNING_CERTIFICATE;
5763
extern const QString CMDLINE_AUTHENTICATE;
5864
extern const QString CMDLINE_SIGN;

‎src/controller/controller.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ void Controller::onCommandHandlerConfirmCompleted(const QVariantMap& res)
233233
_result = res;
234234
writeResponseToStdOut(isInStdinMode, res, commandHandler->commandType());
235235
} catch (const std::exception& error) {
236-
qCritical() << "Command" << std::string(commandType())
236+
qCritical() << "Command" << commandType()
237237
<< "fatal error while writing response to stdout:" << error;
238238
}
239239

@@ -288,8 +288,7 @@ void Controller::onDialogCancel()
288288
void Controller::onCriticalFailure(const QString& error)
289289
{
290290
emit stopCardEventMonitorThread();
291-
qCritical() << "Exiting due to command" << std::string(commandType())
292-
<< "fatal error:" << error;
291+
qCritical() << "Exiting due to command" << commandType() << "fatal error:" << error;
293292
_result =
294293
makeErrorObject(RESP_TECH_ERROR, QStringLiteral("Technical error, see application logs"));
295294
disposeUI();

‎src/controller/retriableerror.cpp‎

Lines changed: 0 additions & 50 deletions
This file was deleted.

‎src/controller/retriableerror.hpp‎

Lines changed: 88 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -26,92 +26,101 @@
2626
#include "pcsc-cpp/pcsc-cpp-utils.hpp"
2727

2828
#include <QMetaType>
29-
#include <QDebug>
3029

31-
enum class RetriableError {
32-
// libpcsc-cpp
33-
SMART_CARD_SERVICE_IS_NOT_RUNNING,
34-
NO_SMART_CARD_READERS_FOUND,
35-
NO_SMART_CARDS_FOUND,
36-
FAILED_TO_COMMUNICATE_WITH_CARD_OR_READER,
37-
SMART_CARD_WAS_REMOVED,
38-
SMART_CARD_TRANSACTION_FAILED,
39-
SCARD_ERROR,
40-
// libelectronic-id
41-
SMART_CARD_CHANGE_REQUIRED,
42-
SMART_CARD_COMMAND_ERROR,
43-
PKCS11_TOKEN_NOT_PRESENT,
44-
PKCS11_TOKEN_REMOVED,
45-
PKCS11_ERROR,
46-
// AutoSelectFailed::Reason
47-
UNSUPPORTED_CARD,
48-
// CertificateReader::run
49-
NO_VALID_CERTIFICATE_AVAILABLE,
50-
PIN_VERIFY_DISABLED,
51-
// default
52-
UNKNOWN_ERROR
53-
};
54-
55-
Q_DECLARE_METATYPE(RetriableError)
30+
class RetriableError
31+
{
32+
Q_GADGET
33+
public:
34+
enum Error : quint8 {
35+
// default
36+
UNKNOWN_ERROR,
37+
// libpcsc-cpp
38+
SMART_CARD_SERVICE_IS_NOT_RUNNING,
39+
NO_SMART_CARD_READERS_FOUND,
40+
NO_SMART_CARDS_FOUND,
41+
FAILED_TO_COMMUNICATE_WITH_CARD_OR_READER,
42+
SMART_CARD_WAS_REMOVED,
43+
SMART_CARD_TRANSACTION_FAILED,
44+
SCARD_ERROR,
45+
// libelectronic-id
46+
SMART_CARD_CHANGE_REQUIRED,
47+
SMART_CARD_COMMAND_ERROR,
48+
PKCS11_TOKEN_NOT_PRESENT,
49+
PKCS11_TOKEN_REMOVED,
50+
PKCS11_ERROR,
51+
// AutoSelectFailed::Reason
52+
UNSUPPORTED_CARD,
53+
// CertificateReader::run
54+
NO_VALID_CERTIFICATE_AVAILABLE,
55+
PIN_VERIFY_DISABLED,
56+
};
57+
Q_ENUM(Error)
5658

57-
QDebug& operator<<(QDebug& d, const RetriableError);
58-
59-
RetriableError toRetriableError(const electronic_id::AutoSelectFailed::Reason reason);
59+
constexpr RetriableError(Error error = UNKNOWN_ERROR) : value(error) {}
60+
constexpr explicit RetriableError(const electronic_id::AutoSelectFailed::Reason reason)
61+
{
62+
switch (reason) {
63+
using enum electronic_id::AutoSelectFailed::Reason;
64+
case SERVICE_NOT_RUNNING:
65+
value = SMART_CARD_SERVICE_IS_NOT_RUNNING;
66+
break;
67+
case NO_READERS:
68+
value = NO_SMART_CARD_READERS_FOUND;
69+
break;
70+
case SINGLE_READER_NO_CARD:
71+
case MULTIPLE_READERS_NO_CARD:
72+
value = NO_SMART_CARDS_FOUND;
73+
break;
74+
case SINGLE_READER_UNSUPPORTED_CARD:
75+
case MULTIPLE_READERS_NO_SUPPORTED_CARD:
76+
value = UNSUPPORTED_CARD;
77+
break;
78+
default:
79+
value = UNKNOWN_ERROR;
80+
}
81+
}
6082

61-
// Define retriable error handling in one place so that it can be reused.
83+
constexpr operator Error() const { return value; }
6284

63-
#define CATCH_PCSC_CPP_RETRIABLE_ERRORS(ERROR_HANDLER) \
64-
catch (const pcsc_cpp::ScardServiceNotRunningError& error) \
65-
{ \
66-
ERROR_HANDLER(RetriableError::SMART_CARD_SERVICE_IS_NOT_RUNNING, error); \
67-
} \
68-
catch (const pcsc_cpp::ScardNoReadersError& error) \
69-
{ \
70-
ERROR_HANDLER(RetriableError::NO_SMART_CARD_READERS_FOUND, error); \
71-
} \
72-
catch (const pcsc_cpp::ScardNoCardError& error) \
73-
{ \
74-
ERROR_HANDLER(RetriableError::NO_SMART_CARDS_FOUND, error); \
75-
} \
76-
catch (const pcsc_cpp::ScardCardCommunicationFailedError& error) \
77-
{ \
78-
ERROR_HANDLER(RetriableError::FAILED_TO_COMMUNICATE_WITH_CARD_OR_READER, error); \
79-
} \
80-
catch (const pcsc_cpp::ScardCardRemovedError& error) \
81-
{ \
82-
ERROR_HANDLER(RetriableError::SMART_CARD_WAS_REMOVED, error); \
83-
} \
84-
catch (const pcsc_cpp::ScardTransactionFailedError& error) \
85-
{ \
86-
ERROR_HANDLER(RetriableError::SMART_CARD_TRANSACTION_FAILED, error); \
87-
} \
88-
catch (const pcsc_cpp::ScardError& error) \
89-
{ \
90-
ERROR_HANDLER(RetriableError::SCARD_ERROR, error); \
85+
static RetriableError catchRetriableError()
86+
{
87+
try {
88+
throw;
89+
} catch (const pcsc_cpp::ScardServiceNotRunningError& /*error*/) {
90+
return SMART_CARD_SERVICE_IS_NOT_RUNNING;
91+
} catch (const pcsc_cpp::ScardNoReadersError& /*error*/) {
92+
return NO_SMART_CARD_READERS_FOUND;
93+
} catch (const pcsc_cpp::ScardNoCardError& /*error*/) {
94+
return NO_SMART_CARDS_FOUND;
95+
} catch (const pcsc_cpp::ScardCardCommunicationFailedError& /*error*/) {
96+
return FAILED_TO_COMMUNICATE_WITH_CARD_OR_READER;
97+
} catch (const pcsc_cpp::ScardCardRemovedError& /*error*/) {
98+
return SMART_CARD_WAS_REMOVED;
99+
} catch (const pcsc_cpp::ScardTransactionFailedError& /*error*/) {
100+
return SMART_CARD_TRANSACTION_FAILED;
101+
} catch (const pcsc_cpp::ScardError& /*error*/) {
102+
return SCARD_ERROR;
103+
} catch (const electronic_id::SmartCardChangeRequiredError& /*error*/) {
104+
return SMART_CARD_CHANGE_REQUIRED;
105+
} catch (const electronic_id::SmartCardError& /*error*/) {
106+
return SMART_CARD_COMMAND_ERROR;
107+
} catch (const electronic_id::Pkcs11TokenNotPresent& /*error*/) {
108+
return PKCS11_TOKEN_NOT_PRESENT;
109+
} catch (const electronic_id::Pkcs11TokenRemoved& /*error*/) {
110+
return PKCS11_TOKEN_REMOVED;
111+
} catch (const electronic_id::Pkcs11Error& /*error*/) {
112+
return PKCS11_ERROR;
113+
} catch (...) {
114+
return UNKNOWN_ERROR;
115+
}
91116
}
92117

93-
#define CATCH_LIBELECTRONIC_ID_RETRIABLE_ERRORS(ERROR_HANDLER) \
94-
catch (const electronic_id::SmartCardChangeRequiredError& error) \
95-
{ \
96-
ERROR_HANDLER(RetriableError::SMART_CARD_CHANGE_REQUIRED, error); \
97-
} \
98-
catch (const electronic_id::SmartCardError& error) \
99-
{ \
100-
ERROR_HANDLER(RetriableError::SMART_CARD_COMMAND_ERROR, error); \
101-
} \
102-
catch (const electronic_id::Pkcs11TokenNotPresent& error) \
103-
{ \
104-
ERROR_HANDLER(RetriableError::PKCS11_TOKEN_NOT_PRESENT, error); \
105-
} \
106-
catch (const electronic_id::Pkcs11TokenRemoved& error) \
107-
{ \
108-
ERROR_HANDLER(RetriableError::PKCS11_TOKEN_REMOVED, error); \
109-
} \
110-
catch (const electronic_id::Pkcs11Error& error) \
111-
{ \
112-
ERROR_HANDLER(RetriableError::PKCS11_ERROR, error); \
113-
}
118+
private:
119+
Error value;
120+
};
114121

115122
#define WARN_RETRIABLE_ERROR(commandType, errorCode, error) \
116123
qWarning().nospace() << "Command " << commandType << " retriable error " << errorCode << ": " \
117124
<< error
125+
126+
Q_DECLARE_METATYPE(RetriableError)

‎src/controller/threads/controllerchildthread.hpp‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@
2323
#pragma once
2424

2525
#include "commandhandler.hpp"
26-
#include "retriableerror.hpp"
27-
#include "qeid.hpp"
2826
#include "logging.hpp"
27+
#include "qeid.hpp"
28+
#include "retriableerror.hpp"
2929

3030
#include <QCoreApplication>
3131
#include <QMutexLocker>
@@ -51,11 +51,7 @@ class ControllerChildThread : public QThread
5151

5252
} catch (const CommandHandlerVerifyPinFailed& error) {
5353
qWarning() << "Command" << commandType() << "PIN verification failed:" << error;
54-
}
55-
CATCH_PCSC_CPP_RETRIABLE_ERRORS(warnAndEmitRetry)
56-
CATCH_LIBELECTRONIC_ID_RETRIABLE_ERRORS(warnAndEmitRetry)
57-
catch (const electronic_id::VerifyPinFailed& error)
58-
{
54+
} catch (const electronic_id::VerifyPinFailed& error) {
5955
switch (error.status()) {
6056
using enum electronic_id::VerifyPinFailed::Status;
6157
case PIN_ENTRY_CANCEL:
@@ -76,19 +72,23 @@ class ControllerChildThread : public QThread
7672
qCritical() << "Command" << commandType() << "fatal error:" << error;
7773
emit failure(error.what());
7874
}
79-
}
80-
catch (const std::exception& error)
81-
{
82-
qCritical() << "Command" << commandType() << "fatal error:" << error;
83-
emit failure(error.what());
75+
} catch (const std::exception& error) {
76+
if (auto errorCode = RetriableError::catchRetriableError();
77+
errorCode != RetriableError::UNKNOWN_ERROR) {
78+
WARN_RETRIABLE_ERROR(commandType(), errorCode, error);
79+
emit retry(errorCode);
80+
} else {
81+
qCritical() << "Command" << commandType() << "fatal error:" << error;
82+
emit failure(error.what());
83+
}
8484
}
8585
}
8686

8787
static QWaitCondition waitForControllerNotify;
8888

8989
signals:
9090
void cancel();
91-
void retry(const RetriableError error);
91+
void retry(RetriableError error);
9292
void failure(const QString& error);
9393

9494
protected:
@@ -111,10 +111,4 @@ class ControllerChildThread : public QThread
111111
private:
112112
virtual void doRun() = 0;
113113
const std::string cmdType;
114-
115-
void warnAndEmitRetry(const RetriableError errorCode, const std::exception& error)
116-
{
117-
WARN_RETRIABLE_ERROR(commandType(), errorCode, error);
118-
emit retry(errorCode);
119-
}
120114
};

0 commit comments

Comments
 (0)