Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/qt/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@
#include <QApplication>
#include <QDebug>
#include <QFontDatabase>
#include <QLatin1String>
#include <QLibraryInfo>
#include <QLocale>
#include <QMessageBox>
#include <QSettings>
#include <QStringBuilder>
#include <QThread>
#include <QTimer>
#include <QTranslator>
Expand Down Expand Up @@ -417,10 +419,23 @@ void BitcoinApplication::shutdownResult()

void BitcoinApplication::handleRunawayException(const QString &message)
{
QMessageBox::critical(nullptr, "Runaway exception", BitcoinGUI::tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) + QString("<br><br>") + message);
QMessageBox::critical(
nullptr, tr("Runaway exception"),
tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) %
QLatin1String("<br><br>") % GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit "qt: Make PACKAGE_BUGREPORT link clickable" (af7e365)

Since our project only uses the issue tracker for development, not user support, it would be good in the future to link to something like stack exchange or a support resources page if a fatal error occurs, instead of the issue tracker.

But this commit is only making existing issue tracker links clickable, not adding new links, so it doesn't cause problems and is good because it keeps the links rendered in consistent way.

In the future it might be be good stop having "Runaway exception" dialogs and instead have distinct "Internal bug" (e.g. unhandled preconditions or postconditions) and "Fatal error" (e.g. failing disk writes) dialogs, treating the first kind like bugs and the second kind like support issues. But it seems good to keep the scope of this PR narrow and focused on the nonfatal checks like it is now.

::exit(EXIT_FAILURE);
}

void BitcoinApplication::handleNonFatalException(const QString& message)
{
assert(QThread::currentThread() == thread());
QMessageBox::warning(
nullptr, tr("Internal error"),
tr("An internal error occurred. %1 will attempt to continue safely. This is "
"an unexpected bug which can be reported as described below.").arg(PACKAGE_NAME) %
QLatin1String("<br><br>") % GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT));
}

WId BitcoinApplication::getMainWinId() const
{
if (!window)
Expand Down
6 changes: 6 additions & 0 deletions src/qt/bitcoin.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ public Q_SLOTS:
/// Handle runaway exceptions. Shows a message box with the problem and quits the program.
void handleRunawayException(const QString &message);

/**
* A helper function that shows a message box
* with details about a non-fatal exception.
*/
void handleNonFatalException(const QString& message);

Q_SIGNALS:
void requestedInitialize();
void requestedShutdown();
Expand Down
2 changes: 1 addition & 1 deletion src/qt/bitcoingui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller)
m_open_wallet_action->setEnabled(true);
m_open_wallet_action->setMenu(m_open_wallet_menu);

connect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet);
GUIUtil::ExceptionSafeConnect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet);
connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet);

for (WalletModel* wallet_model : m_wallet_controller->getOpenWallets()) {
Expand Down
20 changes: 20 additions & 0 deletions src/qt/guiutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include <QGuiApplication>
#include <QJsonObject>
#include <QKeyEvent>
#include <QLatin1String>
#include <QLineEdit>
#include <QList>
#include <QLocale>
Expand All @@ -54,6 +55,7 @@
#include <QShortcut>
#include <QSize>
#include <QString>
#include <QStringBuilder>
#include <QTextDocument> // for Qt::mightBeRichText
#include <QThread>
#include <QUrlQuery>
Expand Down Expand Up @@ -893,4 +895,22 @@ QImage GetImage(const QLabel* label)
#endif
}

QString MakeHtmlLink(const QString& source, const QString& link)
{
return QString(source).replace(
link,
QLatin1String("<a href=\"") % link % QLatin1String("\">") % link % QLatin1String("</a>"));
}

void PrintSlotException(
const std::exception* exception,
const QObject* sender,
const QObject* receiver)
{
std::string description = sender->metaObject()->className();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit "qt: Add GUIUtil::ExceptionSafeConnect function" (f7e260a)

Not important, but just in case you revisit this, I think it might be a little clearer to call this variable "location" instead of "description". (I was a little confused at first about why it didn't seem to include information from the exception.)

description += "->";
description += receiver->metaObject()->className();
PrintExceptionContinue(exception, description.c_str());
}

} // namespace GUIUtil
57 changes: 57 additions & 0 deletions src/qt/guiutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,23 @@
#include <fs.h>
#include <net.h>
#include <netaddress.h>
#include <util/check.h>

#include <QApplication>
#include <QEvent>
#include <QHeaderView>
#include <QItemDelegate>
#include <QLabel>
#include <QMessageBox>
#include <QMetaObject>
#include <QObject>
#include <QProgressBar>
#include <QString>
#include <QTableView>

#include <cassert>
#include <chrono>
#include <utility>

class QValidatedLineEdit;
class SendCoinsRecipient;
Expand Down Expand Up @@ -327,6 +332,58 @@ namespace GUIUtil
QObject::connect(&source, &QObject::destroyed, object, std::forward<Fn>(function), connection);
}

/**
* Replaces a plain text link with an HTML tagged one.
*/
QString MakeHtmlLink(const QString& source, const QString& link);

void PrintSlotException(
const std::exception* exception,
const QObject* sender,
const QObject* receiver);

/**
* A drop-in replacement of QObject::connect function
* (see: https://doc.qt.io/qt-5/qobject.html#connect-3), that
* guaranties that all exceptions are handled within the slot.
*
* NOTE: This function is incompatible with Qt private signals.
*/
template <typename Sender, typename Signal, typename Receiver, typename Slot>
auto ExceptionSafeConnect(
Sender sender, Signal signal, Receiver receiver, Slot method,
Qt::ConnectionType type = Qt::AutoConnection)
{
return QObject::connect(
sender, signal, receiver,
[sender, receiver, method](auto&&... args) {
bool ok{true};
try {
(receiver->*method)(std::forward<decltype(args)>(args)...);
} catch (const NonFatalCheckError& e) {
PrintSlotException(&e, sender, receiver);
ok = QMetaObject::invokeMethod(
qApp, "handleNonFatalException",
blockingGUIThreadConnection(),
Q_ARG(QString, QString::fromStdString(e.what())));
} catch (const std::exception& e) {
PrintSlotException(&e, sender, receiver);
ok = QMetaObject::invokeMethod(
qApp, "handleRunawayException",
blockingGUIThreadConnection(),
Q_ARG(QString, QString::fromStdString(e.what())));
} catch (...) {
PrintSlotException(nullptr, sender, receiver);
ok = QMetaObject::invokeMethod(
qApp, "handleRunawayException",
blockingGUIThreadConnection(),
Q_ARG(QString, "Unknown failure occurred."));
}
assert(ok);
},
type);
}

} // namespace GUIUtil

#endif // BITCOIN_QT_GUIUTIL_H
4 changes: 3 additions & 1 deletion src/qt/sendcoinsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *_platformStyle, QWidget *p
ui->customFee->SetAllowEmpty(false);
ui->customFee->setValue(settings.value("nTransactionFee").toLongLong());
minimizeFeeSection(settings.value("fFeeSectionMinimized").toBool());

GUIUtil::ExceptionSafeConnect(ui->sendButton, &QPushButton::clicked, this, &SendCoinsDialog::sendButtonClicked);
}

void SendCoinsDialog::setClientModel(ClientModel *_clientModel)
Expand Down Expand Up @@ -375,7 +377,7 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa
return true;
}

void SendCoinsDialog::on_sendButton_clicked()
void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
{
if(!model || !model->getOptionsModel())
return;
Expand Down
2 changes: 1 addition & 1 deletion src/qt/sendcoinsdialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public Q_SLOTS:
void updateCoinControlState(CCoinControl& ctrl);

private Q_SLOTS:
void on_sendButton_clicked();
void sendButtonClicked(bool checked);
void on_buttonChooseFee_clicked();
void on_buttonMinimizeFee_clicked();
void removeEntry(SendCoinsEntry* entry);
Expand Down
2 changes: 1 addition & 1 deletion src/qt/test/wallettests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDe
if (status == CT_NEW) txid = hash;
}));
ConfirmSend();
bool invoked = QMetaObject::invokeMethod(&sendCoinsDialog, "on_sendButton_clicked");
bool invoked = QMetaObject::invokeMethod(&sendCoinsDialog, "sendButtonClicked", Q_ARG(bool, false));
assert(invoked);
return txid;
}
Expand Down
4 changes: 2 additions & 2 deletions src/qt/transactionview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa
connect(transactionView, &QTableView::doubleClicked, this, &TransactionView::doubleClicked);
connect(transactionView, &QTableView::customContextMenuRequested, this, &TransactionView::contextualMenu);

connect(bumpFeeAction, &QAction::triggered, this, &TransactionView::bumpFee);
GUIUtil::ExceptionSafeConnect(bumpFeeAction, &QAction::triggered, this, &TransactionView::bumpFee);
connect(abandonAction, &QAction::triggered, this, &TransactionView::abandonTx);
connect(copyAddressAction, &QAction::triggered, this, &TransactionView::copyAddress);
connect(copyLabelAction, &QAction::triggered, this, &TransactionView::copyLabel);
Expand Down Expand Up @@ -424,7 +424,7 @@ void TransactionView::abandonTx()
model->getTransactionTableModel()->updateTransaction(hashQStr, CT_UPDATED, false);
}

void TransactionView::bumpFee()
void TransactionView::bumpFee([[maybe_unused]] bool checked)
{
if(!transactionView || !transactionView->selectionModel())
return;
Expand Down
2 changes: 1 addition & 1 deletion src/qt/transactionview.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private Q_SLOTS:
void openThirdPartyTxUrl(QString url);
void updateWatchOnlyColumn(bool fHaveWatchOnly);
void abandonTx();
void bumpFee();
void bumpFee(bool checked);

Q_SIGNALS:
void doubleClicked(const QModelIndex&);
Expand Down
5 changes: 4 additions & 1 deletion src/qt/walletmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ WalletModel::~WalletModel()
void WalletModel::startPollBalance()
{
// This timer will be fired repeatedly to update the balance
connect(timer, &QTimer::timeout, this, &WalletModel::pollBalanceChanged);
// Since the QTimer::timeout is a private signal, it cannot be used
// in the GUIUtil::ExceptionSafeConnect directly.
connect(timer, &QTimer::timeout, this, &WalletModel::timerTimeout);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to add a comment here why the signal is not directly connected (e.g. because QTimer::timeout is a private signal)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Updated.

GUIUtil::ExceptionSafeConnect(this, &WalletModel::timerTimeout, this, &WalletModel::pollBalanceChanged);
timer->start(MODEL_UPDATE_DELAY);
}

Expand Down
2 changes: 2 additions & 0 deletions src/qt/walletmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ class WalletModel : public QObject
// Notify that there are now keys in the keypool
void canGetAddressesChanged();

void timerTimeout();

public Q_SLOTS:
/* Starts a timer to periodically update the balance */
void startPollBalance();
Expand Down