Remove trailing slash for LastFM auth URLs to avoid a redirect. #6429
Conversation
Also add better error logging while here. This will at least spit out SSLHandshake errors and avoids parsing an empty reply if the request fails. This is fallout from issue #5993
|
This is now ready to be merged, anyone? |
| @@ -174,20 +175,28 @@ void LastFMService::Authenticate() { | |||
| NewClosure(reply, SIGNAL(finished()), this, SLOT(AuthenticateReplyFinished(QNetworkReply*)), reply); | |||
| }); | |||
|
|
|||
| qLog(Debug) << "auth URL:" << url.toString(); | |||
There was a problem hiding this comment.
Can you remove the logging for non-errors here and below?
There was a problem hiding this comment.
I found these very helpful when trying to debug my authentication problem. Note that In case openUrl fails the URL is also being logged. This is also happening just once during the auth phase, so it's not adding undue log volume or anything.
As I said, it was helpful to see what my browser was returning for these URLs and they might help someone to not have to read the code.
Could you expand why you think they should be removed?
There was a problem hiding this comment.
We should avoid logging potentially sensitive data, e.g., tokens, oauth codes, etc. as people often post full logs here.
It's easy enough to add this back in later when developing.
Also add better error logging while here. This will at least spit out
SSLHandshake errors and avoids parsing an empty reply if the request
fails.
This is fallout from issue #5993