Bug 452558 - Spectacle silently dismisses errors raised by purpose sharing plugins
Summary: Spectacle silently dismisses errors raised by purpose sharing plugins
Status: RESOLVED FIXED
Alias: None
Product: Spectacle
Classification: Applications
Component: General (show other bugs)
Version: 21.04.3
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: kde
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-04-12 19:33 UTC by kde
Modified: 2022-04-12 23:08 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description kde 2022-04-12 19:33:57 UTC
SUMMARY

For sharing images to my private image hosting service, I created a purpose plugins which in some instances can raise errors. Currently, spectacle silently dismisses any raised error.

This is how I raise an error:

setError(HttpError);
setErrorText(QString::fromUtf8("Received HTTP Error: ") + statusCode);
emitResult();

where is correctly defined as as a value greater than UserDefinedError:
    enum {
        InvalidFoo = UserDefinedError,
        GenericError,
        NotConnectedError,
        UnknownMimeError,
        WalletFailure,
        HttpError,
    };

I believe the issue dates back to a commit from 3 years ago: https://phabricator.kde.org/D22042

Since error is a boolean, the added code here always silently swallows the error, it will never enter the if (error) at line 419.

I believe the needed change here is to make error an integer which would fit the KJob error enum:
    enum {
        /*** Indicates there is no error */
        NoError = 0,
        /*** Indicates the job was killed */
        KilledJobError = 1,
        /*** Subclasses should define error codes starting at this value */
        UserDefinedError = 100,
    };

This would also need to be changed in the function void ExportMenu::loadPurposeMenu, which receives the error as an integer from the purpose plugin and passes it on as a boolean:

    connect(mPurposeMenu, &Purpose::Menu::finished, this, [this](const QJsonObject &output, int error, const QString &message) {
        if (error) {
            Q_EMIT imageShared(true, message);
        } else {
            Q_EMIT imageShared(false, output[QStringLiteral("url")].toString());
        }
    });

STEPS TO REPRODUCE
1. Share an image via spectacle using a plugin which raises an error via emitResult()

OBSERVED RESULT
No error is raised, the error is silently swallowed.

EXPECTED RESULT
An inline message is shown with the appropriate error message (line 415 in the above commit)

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 
(available in About System)
KDE Plasma Version: 5.24.2
KDE Frameworks Version: 5.91.0
Qt Version:  5.15.2
Comment 1 kde 2022-04-12 22:05:00 UTC
Note: I created a merge request to fix this bug.
Comment 2 Nicolas Fella 2022-04-12 23:05:42 UTC
Git commit aac46ae0609cfa5c531b5407f8091747a98a10ae by Nicolas Fella, on behalf of Daniel Lanz.
Committed on 12/04/2022 at 22:09.
Pushed by nicolasfella into branch 'master'.

Fix silently dismissing user defined errors

All user defined errors were silently dismissed, since
error was passed on as a boolean instead of an integer,
so no differentation could be made anymore down the line.

This fix correctly dismisses a cancelled share but again
shows a user defined error with a red indicator

M  +2    -2    src/Gui/ExportMenu.cpp
M  +1    -1    src/Gui/ExportMenu.h
M  +1    -1    src/Gui/KSMainWindow.cpp
M  +1    -1    src/Gui/KSMainWindow.h

https://invent.kde.org/graphics/spectacle/commit/aac46ae0609cfa5c531b5407f8091747a98a10ae
Comment 3 Nicolas Fella 2022-04-12 23:08:06 UTC
Git commit 0def30320008016f13e3500ab1eb54d4ab9512a1 by Nicolas Fella, on behalf of Daniel Lanz.
Committed on 12/04/2022 at 23:07.
Pushed by nicolasfella into branch 'release/22.04'.

Fix silently dismissing user defined errors

All user defined errors were silently dismissed, since
error was passed on as a boolean instead of an integer,
so no differentation could be made anymore down the line.

This fix correctly dismisses a cancelled share but again
shows a user defined error with a red indicator
(cherry picked from commit aac46ae0609cfa5c531b5407f8091747a98a10ae)

M  +2    -2    src/Gui/ExportMenu.cpp
M  +1    -1    src/Gui/ExportMenu.h
M  +1    -1    src/Gui/KSMainWindow.cpp
M  +1    -1    src/Gui/KSMainWindow.h

https://invent.kde.org/graphics/spectacle/commit/0def30320008016f13e3500ab1eb54d4ab9512a1