Bug 327547

Please upgrade to Amarok 2.8 and then try again, 2.7.1 is just too far away in terms of changes.

It's not easy to reproduce the issue: I can't just let my amarok process not used for two weeks..

In this example of liblastfm they explain a little more the error handling:
https://github.com/lastfm/liblastfm/blob/master/demos/demo2.cpp

private slots:
    void onWsError( lastfm::ws::Error e )
    {
        // QNetworkReply will invoke this slot on application level errors
        // mostly this is only stuff like Ws::InvalidSessionKey and
        // Ws::InvalidApiKey    
        qWarning() << e;
    }


From the liblastfm code (XmlQuery.cpp):
    if ( d->error.enumValue() != lastfm::ws::NoError )
    {
        qDebug() << bytes;

        switch ( d->error.enumValue() )
        {
            case lastfm::ws::OperationFailed:
            case lastfm::ws::InvalidApiKey:
            case lastfm::ws::InvalidSessionKey:
                // NOTE will never be received during the LoginDialog stage
                // since that happens before this slot is registered with
                // QMetaObject in App::App(). Neat :)
                QMetaObject::invokeMethod( qApp, "onWsError", Q_ARG( lastfm::ws::Error, d->error.enumValue() ) );
                break;
            default:
                //do nothing
            break;
        }
    }



And finally from Amarok git master: 
- no onWsError, this explains the log "QMetaObject::invokeMethod: No such method App::onWsError(lastfm::ws::Error)".
- no InvalidSessionKey, so the error code isn't properly handled, so the issue is probably still there.
- error handling in src/services/lastfm/ScrobblerAdapter.cpp only returns a generic error in scrobble(), and logs a warning in slotScrobblesSubmitted(). This hasn't changed much since tag v2.7.1.

In liblastfm (Audioscrobbler.cpp) the api error code is set via mTrack.setScrobbleError(), so we could check for that in ScrobblerAdapter::slotScrobblesSubmitted(), and restart the session there.
Or we could add onWsError to avoid the warning anyway.
liblastfm keeps the tracks in its cache when a temporary error like that happens, so we only need to do a submit() again after the re-authentication.

Maybe other parts of amarok that use liblastfm could gain from that too.

Would you be interested in writing a patch for Amarok to fix these issues?

Yes, if you tell me which solution to implement, as I'm not familiar at all with Amarok source code.

The onWsError could help catch every invalid session key error, not just the scrobbling, and start a re-authentication (but where should I add this code in amarok?)
And  ScrobblerAdapter::slotScrobblesSubmitted could check the error and resubmit in case of Invalid session key, if we know the onWsError has already finished it's re-authentication. Or we could just do a scrobbler.submit() in LastFmService::continueReconfiguring() (it does nothing if the cache is empty, so no network overhead in the usual flow). The later solution seems better to me.

(In reply to comment #4)
> Yes, if you tell me which solution to implement, as I'm not familiar at all
> with Amarok source code.
> 
> The onWsError could help catch every invalid session key error, not just the
> scrobbling, and start a re-authentication (but where should I add this code
> in amarok?)

Please add that method to class App (src/App.h/cpp), guarded by #ifdef HAVE_LIBLASTFM. It would be a little glue that would call appropriate method in ScrobblerAdapter.

> And  ScrobblerAdapter::slotScrobblesSubmitted could check the error and
> resubmit in case of Invalid session key, if we know the onWsError has
> already finished it's re-authentication.

No, above solution is better.

> Or we could just do a scrobbler.submit() in LastFmService::continueReconfiguring() (it does
> nothing if the cache is empty, so no network overhead in the usual flow).

Yes please (I assume continueReconfiguring() gets caller after successful re-auth)

What is the status of this report?

(In reply to Myriam Schweingruber from comment #6)
> What is the status of this report?

I still have this on my to do list but I havn't started yet. I will hopefully have some time in September, but if someone else wants to implement the fix: do it, I don't want to block anything.

No problem, September is coming soon anyway :)

Any updates on this bug? Apparently I have the same bug, but it happens at the start of amarok, and no tracks are scrobbled. I might have started having it after I updated by Kubuntu from 15 to 16, but I am not sure.

Amarok ver 2.8.80

(In reply to Philip from comment #9)
> Any updates on this bug? Apparently I have the same bug, but it happens at
> the start of amarok, and no tracks are scrobbled. I might have started
> having it after I updated by Kubuntu from 15 to 16, but I am not sure.
> 
> Amarok ver 2.8.80

Sadly no, never heard back from the dev who wanted to implement it :-(

Git commit 9c17c0f5af71c1ebe0150039c4d8c3204992158d by Tuomas Nurmi.
Committed on 11/07/2024 at 21:46.
Pushed by nurmi into branch 'master'.

Add general handler for last.fm errors

InvalidSessionKey handled specifically and user is notified, other errors don't
look like having any specific handling for them is necessary.

M  +2    -1    ChangeLog
M  +20   -0    src/App.cpp
M  +9    -0    src/App.h
M  +4    -0    src/services/gpodder/CMakeLists.txt
M  +4    -0    tests/CMakeLists.txt

https://invent.kde.org/multimedia/amarok/-/commit/9c17c0f5af71c1ebe0150039c4d8c3204992158d