Better error handling in KNewStuff backend
ClosedPublic

Authored by leinir on Nov 9 2018, 11:57 AM.

Details

Summary

Previously error handling was done using a string matching method, as that was all KNewStuffCore offered. A newly modified KNewStuffCore error signal (found in D16665) gives much better opportunities to handle error conditions, and this patch modifies the old functionality in Discover's KNewStuff backend to take advantage of this.

BUG: 395937

Diff Detail

Repository
R134 Discover Software Store
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
leinir created this revision.Nov 9 2018, 11:57 AM
Restricted Application added a project: Plasma. · View Herald TranscriptNov 9 2018, 11:57 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
leinir requested review of this revision.Nov 9 2018, 11:57 AM
apol added a comment.Nov 9 2018, 1:05 PM

Where will the All categories are missing error fall right now?

libdiscover/backends/KNSBackend/KNSBackend.cpp
247

Maybe at least qDebug() it?

275

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

leinir marked 2 inline comments as done.Nov 12 2018, 11:37 AM
In D16776#356741, @apol wrote:

Where will the All categories are missing error fall right now?

That is a configuration file error (which is the case, unless there's a network fault, which should be reported earlier). There is a corner case where the server might be broken and report all categories missing, even if they aren't (such as them having been unexported for some reason or another), but we can't detect that on the client, since from the perspective of a client, that is just the same thing as the category not existing. So, ConfigFileError catches that :)

libdiscover/backends/KNSBackend/KNSBackend.cpp
247

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)

275

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 updated this revision to Diff 45355.Nov 12 2018, 11:39 AM
leinir marked 2 inline comments as done.

Address @apol's comments

apol accepted this revision.Nov 12 2018, 11:52 AM

Thanks!

This revision is now accepted and ready to land.Nov 12 2018, 11:52 AM
This revision was automatically updated to reflect the committed changes.