Differential D16776 Diff 45398 libdiscover/backends/KNSBackend/KNSBackend.cpp

Changeset View

Standalone View

View Options

libdiscover/backends/KNSBackend/KNSBackend.cpp

Show First 20 Lines • Show All 107 Lines • ▼ Show 20 Line(s)94{
108 m_extends = group.readEntry("Extends", QStringList()); 108​ m_extends = group.readEntry("Extends", QStringList());
109 m_reviews->setProviderUrl(QUrl(group.readEntry("ProvidersUrl", QString()))); 109​ m_reviews->setProviderUrl(QUrl(group.readEntry("ProvidersUrl", QString())));
110 110​
111 setFetching(true); 111​ setFetching(true);
112 112​
113 m_engine = new KNSCore::Engine(this); 113​ m_engine = new KNSCore::Engine(this);
114 m_engine->init(m_name); 114​ m_engine->init(m_name);
115 m_engine->setPageSize(100); 115​ m_engine->setPageSize(100);
116 // Setting setFetching to false when we get an error ensures we don't end up in an eternally-fetching state 116​ connect(m_engine, &KNSCore::Engine::signalErrorCode, this, &KNSBackend::signalErrorCode);
117 connect(m_engine, &KNSCore::Engine::signalError, this, [this](const QString &_error) { ​
118 QString error = _error; ​
119 bool invalidFile = false; ​
120 if(error == QLatin1Literal("All categories are missing")) { ​
121 markInvalid(error); ​
122 error = i18n("Invalid %1 backend, contact your distributor.", m_displayName); ​
123 invalidFile = true; ​
124 } ​
125 m_responsePending = false; ​
126 Q_EMIT searchFinished(); ​
127 Q_EMIT availableForQueries(); ​
128 this->setFetching(false); ​
129 qWarning() << "kns error" << objectName() << error; ​
130 if (!invalidFile) ​
131 Q_EMIT passiveMessage(i18n("%1: %2", name(), error)); ​
132 }); ​
133 connect(m_engine, &KNSCore::Engine::signalEntriesLoaded, this, &KNSBackend::receivedEntries, Qt::QueuedConnection); 117​ connect(m_engine, &KNSCore::Engine::signalEntriesLoaded, this, &KNSBackend::receivedEntries, Qt::QueuedConnection);
134 connect(m_engine, &KNSCore::Engine::signalEntryChanged, this, &KNSBackend::statusChanged, Qt::QueuedConnection); 118​ connect(m_engine, &KNSCore::Engine::signalEntryChanged, this, &KNSBackend::statusChanged, Qt::QueuedConnection);
135 connect(m_engine, &KNSCore::Engine::signalEntryDetailsLoaded, this, &KNSBackend::statusChanged); 119​ connect(m_engine, &KNSCore::Engine::signalEntryDetailsLoaded, this, &KNSBackend::statusChanged);
136 connect(m_engine, &KNSCore::Engine::signalProvidersLoaded, this, &KNSBackend::fetchInstalled); 120​ connect(m_engine, &KNSCore::Engine::signalProvidersLoaded, this, &KNSBackend::fetchInstalled);
137 121​
138 const QVector<QPair<FilterType, QString>> filters = { {CategoryFilter, fileName } }; 122​ const QVector<QPair<FilterType, QString>> filters = { {CategoryFilter, fileName } };
139 const QSet<QString> backendName = { name() }; 123​ const QSet<QString> backendName = { name() };
140 m_displayName = group.readEntry("Name", QString()); 124​ m_displayName = group.readEntry("Name", QString());
▲ Show 20 Lines • Show All 111 Lines • ▼ Show 20 Line(s)228{
252 m_engine->requestMoreData(); 236​ m_engine->requestMoreData();
253} 237​}
254 238​
255void KNSBackend::statusChanged(const KNSCore::EntryInternal& entry) 239​void KNSBackend::statusChanged(const KNSCore::EntryInternal& entry)
256{ 240​{
257 resourceForEntry(entry); 241​ resourceForEntry(entry);
258} 242​}
259 243​
244​void KNSBackend::signalErrorCode(const KNSCore::ErrorCode& errorCode, const QString& message, const QVariant& metadata)
245​{
246​ QString error = message;
247​ qDebug() << "KNS backend error in backend" << m_displayName << ":" << errorCode << message << metadata;
apolUnsubmitted
Done

Maybe at least qDebug() it?

apol: Maybe at least qDebug() it?
leinirAuthorUnsubmitted
Not Done

guessing the "it" is the error message referred to in the comment below? At any rate, yes, debugging it out is not a bad idea at all, i'll do that :) (i do notice that there's very few qCDebugs going on... this might want fixing, but that's a bit outside the scope of this patch, of course)

leinir: guessing the "it" is the error message referred to in the comment below? At any rate, yes…
248​ bool invalidFile = false;
249​ switch(errorCode) {
250​ case KNSCore::ErrorCode::UnknownError:
251​ // This is not supposed to be hit, of course, but any error coming to this point should be non-critical and safely ignored.
252​ break;
253​ case KNSCore::ErrorCode::NetworkError:
254​ // If we have a network error, we need to tell the user about it. This is almost always fatal, so mark invalid and tell the user.
255​ error = i18n("Network error in backend %1: %2", m_displayName, metadata.toInt());
256​ markInvalid(error);
257​ invalidFile = true;
258​ break;
259​ case KNSCore::ErrorCode::OcsError:
260​ if(metadata.toInt() == 200) {
261​ // Too many requests, try again in a couple of minutes - perhaps we can simply postpone it automatically, and give a message?
262​ error = i18n("Too many requests sent to the server for backend %1. Please try again in a few minutes.", m_displayName);
263​ } else {
264​ // Unknown API error, usually something critical, mark as invalid and cry a lot
265​ error = i18n("Invalid %1 backend, contact your distributor.", m_displayName);
266​ markInvalid(error);
267​ invalidFile = true;
268​ }
269​ break;
270​ case KNSCore::ErrorCode::ConfigFileError:
271​ error = i18n("Invalid %1 backend, contact your distributor.", m_displayName);
272​ markInvalid(error);
273​ invalidFile = true;
274​ break;
275​ case KNSCore::ErrorCode::ProviderError:
apolUnsubmitted
Done

Are you setting error after passing error? This reads weird. If you want to pass an empty one use markInvalid({}).

apol: Are you setting error after passing error? This reads weird. If you want to pass an empty one…
leinirAuthorUnsubmitted
Not Done

really only did this because it was how it was done in the old version of the function... But yes, it does kind of read weird, i'll swap them around to make it read more sensibly

leinir: really only did this because it was how it was done in the old version of the function... But…
276​ error = i18n("Invalid %1 backend, contact your distributor.", m_displayName);
277​ markInvalid(error);
278​ invalidFile = true;
279​ break;
280​ case KNSCore::ErrorCode::InstallationError:
281​ // This error is handled already, by forwarding the KNS engine's installer error message.
282​ break;
283​ case KNSCore::ErrorCode::ImageError:
284​ // Image fetching errors are not critical as such, but may lead to weird layout issues, might want handling...
285​ error = i18n("Could not fetch screenshot for the entry %1 in backend %2", metadata.toList().at(0).toString(), m_displayName);
286​ break;
287​ default:
288​ // Having handled all current error values, we should by all rights never arrive here, but for good order and future safety...
289​ error = i18n("Unhandled error in %1 backend. Contact your distributor.", m_displayName);
290​ break;
291​ }
292​ m_responsePending = false;
293​ Q_EMIT searchFinished();
294​ Q_EMIT availableForQueries();
295​ // Setting setFetching to false when we get an error ensures we don't end up in an eternally-fetching state
296​ this->setFetching(false);
297​ qWarning() << "kns error" << objectName() << error;
298​ if (!invalidFile)
299​ Q_EMIT passiveMessage(i18n("%1: %2", name(), error));
300​}
301​
260class KNSTransaction : public Transaction 302​class KNSTransaction : public Transaction
261{ 303​{
262public: 304​public:
263 KNSTransaction(QObject* parent, KNSResource* res, Transaction::Role role) 305​ KNSTransaction(QObject* parent, KNSResource* res, Transaction::Role role)
264 : Transaction(parent, res, role) 306​ : Transaction(parent, res, role)
265 , m_id(res->entry().uniqueId()) 307​ , m_id(res->entry().uniqueId())
266 { 308​ {
267 setCancellable(false); 309​ setCancellable(false);
▲ Show 20 Lines • Show All 201 Lines • Show Last 20 Lines