Refactor/improve error handling for opening/saving cloud scores

This commit is contained in:
Casper Jeukendrup 2023-08-28 15:42:18 +02:00
parent 50065f72f1
commit 733746d65a
No known key found for this signature in database
GPG key ID: 6C571BEF59E722DD
5 changed files with 169 additions and 96 deletions

View file

@ -34,9 +34,16 @@ enum class Err {
UnknownError = int(Ret::Code::CloudFirst),
AccessTokenIsEmpty,
AccountNotActivated,
Conflict,
NetworkError, /// < use cloudNetworkErrorUserDescription to retrieve user-readable description
Status400_InvalidRequest,
Status401_AuthorizationRequired,
Status403_AccountNotActivated,
Status403_NotOwner,
Status404_NotFound,
Status409_Conflict,
Status422_ValidationFailed,
Status500_InternalServerError,
UnknownStatusCode,
NetworkError,
CouldNotReceiveSourceUrl,
};
@ -49,25 +56,21 @@ inline Ret make_ret(Err e)
case Err::NoError: return Ret(retCode);
case Err::UnknownError: return Ret(retCode);
case Err::AccessTokenIsEmpty: return Ret(retCode, "Access token is empty");
case Err::AccountNotActivated: return Ret(retCode, "Account not activated");
case Err::Conflict: return Ret(retCode, "Conflict");
case Err::Status400_InvalidRequest: return Ret(retCode, "Status 400: invalid request");
case Err::Status401_AuthorizationRequired: return Ret(retCode, "Status 401: authorization required");
case Err::Status403_AccountNotActivated: return Ret(retCode, "Status 403: account not activated");
case Err::Status403_NotOwner: return Ret(retCode, "Status 403: not owner");
case Err::Status404_NotFound: return Ret(retCode, "Status 404: not found");
case Err::Status409_Conflict: return Ret(retCode, "Status 409: conflict");
case Err::Status422_ValidationFailed: return Ret(retCode, "Status 422: validation failed");
case Err::Status500_InternalServerError: return Ret(retCode, "Status 500: internal server error");
case Err::UnknownStatusCode: return Ret(retCode, "Unknown status code");
case Err::NetworkError: return Ret(retCode, "Network error");
case Err::CouldNotReceiveSourceUrl: return Ret(retCode, "Could not receive source url");
break;
}
return Ret(static_cast<int>(e));
}
inline std::string cloudNetworkErrorUserDescription(const Ret& ret)
{
assert(ret.code() == int(Err::NetworkError));
auto userDescription = ret.data(CLOUD_NETWORK_ERROR_USER_DESCRIPTION_KEY);
if (userDescription.has_value()) {
return std::any_cast<std::string>(userDescription);
}
return {};
return Ret(retCode);
}
}

View file

@ -366,37 +366,38 @@ Ret AbstractCloudService::uploadingDownloadingRetFromRawRet(const Ret& rawRet, b
return rawRet; // OK
}
int code = statusCode(rawRet);
if (!code) {
return rawRet;
int status = statusCode(rawRet);
if (status) {
switch (status) {
case 400: return make_ret(Err::Status400_InvalidRequest);
case 401: return make_ret(Err::Status401_AuthorizationRequired);
case 403:
if (isAlreadyUploaded) {
return make_ret(Err::Status403_AccountNotActivated);
} else {
return make_ret(Err::Status403_NotOwner);
}
case 404: return make_ret(Err::Status404_NotFound);
case 409: return make_ret(Err::Status409_Conflict);
case 422: return make_ret(Err::Status422_ValidationFailed);
case 500: return make_ret(Err::Status500_InternalServerError);
default: break;
}
Ret ret = make_ret(Err::UnknownStatusCode);
ret.setText("Unknown status code: " + std::to_string(status));
ret.setData("status", status);
return ret;
}
if (!isAlreadyUploaded && code == FORBIDDEN_CODE) {
return make_ret(cloud::Err::AccountNotActivated);
switch (rawRet.code()) {
case int(network::Err::NetworkError):
case int(network::Err::Timeout):
case int(network::Err::Abort):
return make_ret(Err::NetworkError);
}
if (code == CONFLICT_STATUS_CODE) {
return make_ret(cloud::Err::Conflict);
}
static const std::map<int, mu::TranslatableString> codes {
{ 400, mu::TranslatableString("cloud", "Invalid request") },
{ 401, mu::TranslatableString("cloud", "Authorization required") },
{ 403, mu::TranslatableString("cloud", "Forbidden. User is not owner of the score.") },
{ 422, mu::TranslatableString("cloud", "Validation failed") },
{ 500, mu::TranslatableString("cloud", "Internal server error") },
};
std::string userDescription = qtrc("cloud", "Error %1: %2")
.arg(code)
.arg(mu::value(codes, code, TranslatableString("cloud", "Unknown error")).qTranslated())
.toStdString();
Ret ret = make_ret(cloud::Err::NetworkError);
ret.setData(CLOUD_NETWORK_ERROR_USER_DESCRIPTION_KEY, userDescription);
ret.setData(STATUS_KEY, code);
return ret;
return rawRet;
}
int AbstractCloudService::statusCode(const mu::Ret& ret) const

View file

@ -215,7 +215,7 @@ mu::RetVal<ScoreInfo> MuseScoreComService::downloadScoreInfo(int scoreId)
if (!ret) {
printServerReply(receivedData);
result.ret = ret;
result.ret = uploadingDownloadingRetFromRawRet(ret);
return result;
}

View file

@ -455,8 +455,7 @@ Ret ProjectActionsController::openScoreFromMuseScoreCom(const QUrl& url)
bool isOwner = true;
RetVal<cloud::ScoreInfo> scoreInfo = museScoreComService()->downloadScoreInfo(scoreId);
if (!scoreInfo.ret) {
std::any status = scoreInfo.ret.data("status");
if (status.has_value() && std::any_cast<int>(status) == 403) {
if (scoreInfo.ret.code() == int(cloud::Err::Status403_NotOwner)) {
LOGI() << "Downloading score info returned 403, so not owner";
isOwner = false;
} else {
@ -1497,38 +1496,60 @@ void ProjectActionsController::showScoreDownloadError(const Ret& ret)
message = trc("project", "This score is invalid.");
break;
case int(Err::FileOpenError):
message = trc("project", "The file could not be downloaded to your disk.");
message = trc("project/cloud", "The file could not be downloaded to your disk.");
break;
case int(network::Err::NetworkError): {
case int(cloud::Err::Status400_InvalidRequest):
//: %1 will be replaced with the error code that MuseScore.com returned; this might contain english text
//: that is deliberately not translated
message = qtrc("project/cloud", "MuseScore.com returned an error code: %1.")
.arg("400 Invalid request").toStdString();
break;
case int(cloud::Err::Status401_AuthorizationRequired):
//: %1 will be replaced with the error code that MuseScore.com returned; this might contain english text
//: that is deliberately not translated
message = qtrc("project/cloud", "MuseScore.com returned an error code: %1.")
.arg("401 Authorization required").toStdString();
break;
case int(cloud::Err::Status403_AccountNotActivated):
message = trc("project/cloud", "Your musescore.com account needs to be verified first. "
"Please activate your account via the link in the activation email.");
break;
case int(cloud::Err::Status403_NotOwner):
message = trc("project/cloud", "This score does not belong to this account. To access this score, make sure you are logged in "
"to the desktop app with the account to which this score belongs.");
break;
case int(cloud::Err::Status404_NotFound):
message = trc("project/cloud", "The score could not be found, or cannot be accessed by your account.");
break;
case int(cloud::Err::Status422_ValidationFailed):
//: %1 will be replaced with the error code that MuseScore.com returned; this might contain english text
//: that is deliberately not translated
message = qtrc("project/cloud", "MuseScore.com returned an error code: %1.")
.arg("422 Validation failed").toStdString();
break;
case int(cloud::Err::Status500_InternalServerError):
//: %1 will be replaced with the error code that MuseScore.com returned; this might contain english text
//: that is deliberately not translated
message = qtrc("project/cloud", "MuseScore.com returned an error code: %1.")
.arg("500 Internal server error").toStdString();
break;
case int(cloud::Err::UnknownStatusCode): {
std::any status = ret.data("status");
if (status.has_value()) {
int statusCode = std::any_cast<int>(status);
switch (statusCode) {
case 404:
message = trc("project", "The score could not be found, or cannot be accessed by your account.");
break;
default:
message = qtrc("project", "<a href=\"https://musescore.com\">musescore.com</a> returned error code %1.")
.arg(statusCode).toStdString();
}
//: %1 will be replaced with the error code that MuseScore.com returned, which is a number.
message = qtrc("project/cloud", "MuseScore.com returned an unknown error code: %1.")
.arg(std::any_cast<int>(status)).toStdString();
} else {
message = trc("project", "Could not connect to <a href=\"https://musescore.com\">musescore.com</a>. "
"Please check your internet connection or try again later.");
message = trc("project/cloud", "MuseScore.com returned an unknown error code.");
}
} break;
case int(cloud::Err::AccountNotActivated):
message = trc("project", "Your musescore.com account needs to be verified first. "
"Please activate your account via the link in the activation email.");
break;
case int(cloud::Err::NetworkError):
message = cloud::cloudNetworkErrorUserDescription(ret);
if (!message.empty()) {
message += "\n\n" + trc("project/save", "Please try again later.");
break;
}
// FALLTHROUGH
message = trc("project/cloud", "Could not connect to <a href=\"https://musescore.com\">musescore.com</a>. "
"Please check your internet connection or try again later.");
break;
default:
message = trc("project/save", "Please try again later.");
message = trc("project/cloud", "Please try again later.");
break;
}

View file

@ -246,12 +246,16 @@ RetVal<CloudProjectInfo> SaveProjectScenario::doAskCloudLocation(INotationProjec
}
break;
case int(cloud::Err::AccountNotActivated):
case int(cloud::Err::Status400_InvalidRequest):
case int(cloud::Err::Status403_AccountNotActivated):
case int(cloud::Err::Status422_ValidationFailed):
case int(cloud::Err::Status500_InternalServerError):
case int(cloud::Err::UnknownStatusCode):
case int(cloud::Err::NetworkError):
return showCloudSaveError(scoreInfo.ret, project->cloudInfo(), isPublish, false);
// It's possible the source URL is invalid or points to a score on a different user's account.
// In this situation we shouldn't see an error.
// In this situation we shouldn't show an error.
default: break;
}
}
@ -422,12 +426,26 @@ Ret SaveProjectScenario::showCloudSaveError(const Ret& ret, const CloudProjectIn
int defaultButtonCode = okBtn.btn;
switch (ret.code()) {
case int(cloud::Err::AccountNotActivated):
msg = trc("project/save", "Your musescore.com account needs to be verified first. "
"Please activate your account via the link in the activation email.");
case int(cloud::Err::Status400_InvalidRequest):
//: %1 will be replaced with the error code that MuseScore.com returned; this might contain english text
//: that is deliberately not translated
msg = qtrc("project/cloud", "MuseScore.com returned an error code: %1.")
.arg("400 Invalid request").toStdString();
msg += "\n\n" + trc("project/cloud", "Please try again later, or get help for this problem on musescore.org.");
break;
case int(cloud::Err::Status401_AuthorizationRequired):
//: %1 will be replaced with the error code that MuseScore.com returned; this might contain english text
//: that is deliberately not translated
msg = qtrc("project/cloud", "MuseScore.com returned an error code: %1.")
.arg("400 Authorization required").toStdString();
msg += "\n\n" + trc("project/cloud", "Please try again later, or get help for this problem on musescore.org.");
break;
case int(cloud::Err::Status403_AccountNotActivated):
msg = trc("project/cloud", "Your musescore.com account needs to be verified first. "
"Please activate your account via the link in the activation email.");
buttons = { okBtn };
break;
case int(cloud::Err::Conflict):
case int(cloud::Err::Status409_Conflict):
title = trc("project/save", "There are conflicting changes in the online score");
if (isPublish) {
msg = qtrc("project/save", "You can replace the <a href=\"%1\">online score</a>, or publish this as a new score "
@ -453,15 +471,37 @@ Ret SaveProjectScenario::showCloudSaveError(const Ret& ret, const CloudProjectIn
defaultButtonCode = replaceBtnCode;
}
break;
case int(cloud::Err::NetworkError):
msg = cloud::cloudNetworkErrorUserDescription(ret);
if (!msg.empty()) {
msg += "\n\n" + trc("project/save", "Please try again later, or get help for this problem on musescore.org.");
break;
case int(cloud::Err::Status422_ValidationFailed):
//: %1 will be replaced with the error code that MuseScore.com returned; this might contain english text
//: that is deliberately not translated
msg = qtrc("project/cloud", "MuseScore.com returned an error code: %1.")
.arg("422 Validation failed").toStdString();
msg += "\n\n" + trc("project/cloud", "Please try again later, or get help for this problem on musescore.org.");
break;
case int(cloud::Err::Status500_InternalServerError):
//: %1 will be replaced with the error code that MuseScore.com returned; this might contain english text
//: that is deliberately not translated
msg = qtrc("project/cloud", "MuseScore.com returned an error code: %1.")
.arg("500 Internal server error").toStdString();
msg += "\n\n" + trc("project/cloud", "Please try again later, or get help for this problem on musescore.org.");
break;
case int(cloud::Err::UnknownStatusCode): {
std::any status = ret.data("status");
if (status.has_value()) {
//: %1 will be replaced with the error code that MuseScore.com returned, which is a number.
msg = qtrc("project/cloud", "MuseScore.com returned an unknown error code: %1.")
.arg(std::any_cast<int>(status)).toStdString();
} else {
msg = trc("project/cloud", "MuseScore.com returned an unknown error code.");
}
// FALLTHROUGH
msg += "\n\n" + trc("project/cloud", "Please try again later, or get help for this problem on musescore.org.");
} break;
case int(cloud::Err::NetworkError):
msg = trc("project/cloud", "Could not connect to <a href=\"https://musescore.com\">musescore.com</a>. "
"Please check your internet connection or try again later.");
break;
default:
msg = trc("project/save", "Please try again later, or get help for this problem on musescore.org.");
msg = trc("project/cloud", "Please try again later, or get help for this problem on musescore.org.");
break;
}
@ -485,26 +525,34 @@ Ret SaveProjectScenario::showCloudSaveError(const Ret& ret, const CloudProjectIn
Ret SaveProjectScenario::showAudioCloudShareError(const Ret& ret) const
{
std::string title= trc("project/save", "Your audio could not be shared");
std::string title= trc("project/share", "Your audio could not be shared");
std::string msg;
IInteractive::ButtonData okBtn = interactive()->buttonData(IInteractive::Button::Ok);
IInteractive::ButtonDatas buttons = IInteractive::ButtonDatas { okBtn };
switch (ret.code()) {
case int(cloud::Err::AccountNotActivated):
msg = trc("project/save", "Your audio.com account needs to be verified first. "
"Please activate your account via the link in the activation email.");
case int(cloud::Err::Status403_AccountNotActivated):
msg = trc("project/share", "Your audio.com account needs to be verified first. "
"Please activate your account via the link in the activation email.");
break;
case int(cloud::Err::NetworkError):
msg = cloud::cloudNetworkErrorUserDescription(ret);
if (!msg.empty()) {
msg += "\n\n" + trc("project/save", "Please try again later, or get help for this problem on audio.com.");
break;
case int(cloud::Err::UnknownStatusCode): {
std::any status = ret.data("status");
if (status.has_value()) {
//: %1 will be replaced with the error code that audio.com returned, which is a number.
msg = qtrc("project/share", "Audio.com returned an unknown error code: %1.")
.arg(std::any_cast<int>(status)).toStdString();
} else {
msg = trc("project/share", "Audio.com returned an unknown error code.");
}
// FALLTHROUGH
msg += "\n\n" + trc("project/share", "Please try again later, or get help for this problem on audio.com.");
} break;
case int(cloud::Err::NetworkError):
msg = trc("project/share", "Could not connect to audio.com. "
"Please check your internet connection or try again later.");
break;
default:
msg = trc("project/save", "Please try again later, or get help for this problem on audio.com.");
msg = trc("project/share", "Please try again later, or get help for this problem on audio.com.");
break;
}