From 385fe21d16bd16f0d49f29e30629914584abcf99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kuba=20Szczodrzy=C5=84ski?= Date: Tue, 5 Nov 2019 11:27:29 +0100 Subject: [PATCH] [APIv2/Librus] Implement error handling. Catch exceptions in ApiService. --- .../edziennik/api/v2/ApiService.kt | 13 ++- .../szczodrzynski/edziennik/api/v2/Errors.kt | 1 + .../edziennik/api/v2/librus/Librus.kt | 96 ++++++++++++++++--- .../api/v2/librus/data/LibrusPortal.kt | 5 +- .../edziennik/api/v2/models/Data.kt | 2 - .../data/db/modules/login/LoginStore.java | 11 ++- 6 files changed, 101 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/ApiService.kt b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/ApiService.kt index 66ea5b1e..162ecde1 100644 --- a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/ApiService.kt +++ b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/ApiService.kt @@ -90,6 +90,7 @@ class ApiService : Service() { errorList.add(apiError) apiError.throwable?.printStackTrace() if (apiError.isCritical) { + taskRunning?.cancel() notification.setCriticalError().post() taskRunning = null taskIsRunning = false @@ -154,10 +155,14 @@ class ApiService : Service() { // post an event EventBus.getDefault().post(ApiTaskStartedEvent(taskProfileId, task.profile)) - when (task) { - is EdziennikTask -> task.run(app, taskCallback) - is NotifyTask -> task.run(app, taskCallback) - is ErrorReportTask -> task.run(app, taskCallback, notification, errorList) + try { + when (task) { + is EdziennikTask -> task.run(app, taskCallback) + is NotifyTask -> task.run(app, taskCallback) + is ErrorReportTask -> task.run(app, taskCallback, notification, errorList) + } + } catch (e: Exception) { + taskCallback.onError(ApiError(TAG, EXCEPTION_API_TASK).withThrowable(e)) } } diff --git a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/Errors.kt b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/Errors.kt index f1f3564c..c2daa722 100644 --- a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/Errors.kt +++ b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/Errors.kt @@ -150,6 +150,7 @@ const val ERROR_IDZIENNIK_API_OTHER = 451 const val ERROR_TEMPLATE_WEB_OTHER = 801 +const val EXCEPTION_API_TASK = 900 const val EXCEPTION_LOGIN_LIBRUS_API_TOKEN = 901 const val EXCEPTION_LOGIN_LIBRUS_PORTAL_TOKEN = 902 const val EXCEPTION_LIBRUS_PORTAL_SYNERGIA_TOKEN = 903 diff --git a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/librus/Librus.kt b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/librus/Librus.kt index 937fe1a6..eeb2b759 100644 --- a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/librus/Librus.kt +++ b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/librus/Librus.kt @@ -5,7 +5,7 @@ package pl.szczodrzynski.edziennik.api.v2.librus import pl.szczodrzynski.edziennik.App -import pl.szczodrzynski.edziennik.api.v2.CODE_INTERNAL_LIBRUS_ACCOUNT_410 +import pl.szczodrzynski.edziennik.api.v2.* import pl.szczodrzynski.edziennik.api.v2.interfaces.EdziennikCallback import pl.szczodrzynski.edziennik.api.v2.interfaces.EdziennikInterface import pl.szczodrzynski.edziennik.api.v2.librus.data.LibrusData @@ -14,9 +14,7 @@ import pl.szczodrzynski.edziennik.api.v2.librus.firstlogin.LibrusFirstLogin import pl.szczodrzynski.edziennik.api.v2.librus.login.LibrusLogin import pl.szczodrzynski.edziennik.api.v2.librus.login.LibrusLoginApi import pl.szczodrzynski.edziennik.api.v2.librus.login.LibrusLoginSynergia -import pl.szczodrzynski.edziennik.api.v2.librusLoginMethods import pl.szczodrzynski.edziennik.api.v2.models.ApiError -import pl.szczodrzynski.edziennik.api.v2.prepare import pl.szczodrzynski.edziennik.data.db.modules.login.LoginStore import pl.szczodrzynski.edziennik.data.db.modules.profiles.Profile import pl.szczodrzynski.edziennik.utils.Utils.d @@ -53,12 +51,28 @@ class Librus(val app: App, val profile: Profile?, val loginStore: LoginStore, va |__*/ override fun sync(featureIds: List, viewId: Int?) { data.prepare(librusLoginMethods, LibrusFeatures, featureIds, viewId) - d(TAG, "LoginMethod IDs: ${data.targetLoginMethodIds}") - d(TAG, "Endpoint IDs: ${data.targetEndpointIds}") + login() + } + + private fun login() { + d(TAG, "Trying to login with ${data.targetLoginMethodIds}") + if (internalErrorList.isNotEmpty()) { + d(TAG, " - Internal errors:") + internalErrorList.forEach { d(TAG, " - code $it") } + } LibrusLogin(data) { - LibrusData(data) { - completed() - } + data() + } + } + + private fun data() { + d(TAG, "Endpoint IDs: ${data.targetEndpointIds}") + if (internalErrorList.isNotEmpty()) { + d(TAG, " - Internal errors:") + internalErrorList.forEach { d(TAG, " - code $it") } + } + LibrusData(data) { + completed() } } @@ -102,15 +116,67 @@ class Librus(val app: App, val profile: Profile?, val loginStore: LoginStore, va } override fun onError(apiError: ApiError) { + if (apiError.errorCode in internalErrorList) { + // finish immediately if the same error occurs twice during the same sync + callback.onError(apiError) + return + } + internalErrorList.add(apiError.errorCode) when (apiError.errorCode) { - in internalErrorList -> { - // finish immediately if the same error occurs twice during the same sync - callback.onError(apiError) + ERROR_LIBRUS_PORTAL_ACCESS_DENIED -> { + data.loginMethods.remove(LOGIN_METHOD_LIBRUS_PORTAL) + data.targetLoginMethodIds.add(LOGIN_METHOD_LIBRUS_PORTAL) + data.targetLoginMethodIds.sort() + data.portalTokenExpiryTime = 0 + login() } - CODE_INTERNAL_LIBRUS_ACCOUNT_410 -> { - internalErrorList.add(apiError.errorCode) - loginStore.removeLoginData("refreshToken") // force a clean login - //loginLibrus() + ERROR_LIBRUS_API_ACCESS_DENIED, + ERROR_LIBRUS_API_TOKEN_EXPIRED -> { + data.loginMethods.remove(LOGIN_METHOD_LIBRUS_API) + data.targetLoginMethodIds.add(LOGIN_METHOD_LIBRUS_API) + data.targetLoginMethodIds.sort() + data.apiTokenExpiryTime = 0 + login() + } + ERROR_LIBRUS_SYNERGIA_ACCESS_DENIED -> { + data.loginMethods.remove(LOGIN_METHOD_LIBRUS_SYNERGIA) + data.targetLoginMethodIds.add(LOGIN_METHOD_LIBRUS_SYNERGIA) + data.targetLoginMethodIds.sort() + data.synergiaSessionIdExpiryTime = 0 + login() + } + ERROR_LIBRUS_MESSAGES_ACCESS_DENIED -> { + data.loginMethods.remove(LOGIN_METHOD_LIBRUS_MESSAGES) + data.targetLoginMethodIds.add(LOGIN_METHOD_LIBRUS_MESSAGES) + data.targetLoginMethodIds.sort() + data.messagesSessionIdExpiryTime = 0 + login() + } + ERROR_LOGIN_LIBRUS_PORTAL_NO_CODE, + ERROR_LOGIN_LIBRUS_PORTAL_CSRF_MISSING, + ERROR_LOGIN_LIBRUS_PORTAL_CODE_REVOKED, + ERROR_LOGIN_LIBRUS_PORTAL_CODE_EXPIRED -> { + login() + } + ERROR_LOGIN_LIBRUS_PORTAL_NO_REFRESH, + ERROR_LOGIN_LIBRUS_PORTAL_REFRESH_REVOKED, + ERROR_LOGIN_LIBRUS_PORTAL_REFRESH_INVALID -> { + data.portalRefreshToken = null + login() + } + ERROR_LOGIN_LIBRUS_SYNERGIA_TOKEN_INVALID, + ERROR_LOGIN_LIBRUS_SYNERGIA_NO_TOKEN, + ERROR_LOGIN_LIBRUS_SYNERGIA_NO_SESSION_ID -> { + login() + } + ERROR_LOGIN_LIBRUS_MESSAGES_NO_SESSION_ID -> { + login() + } + // TODO PORTAL CAPTCHA + ERROR_LIBRUS_API_TIMETABLE_NOT_PUBLIC, + ERROR_LIBRUS_API_LUCKY_NUMBER_NOT_ACTIVE, + ERROR_LIBRUS_API_NOTES_NOT_ACTIVE -> { + data() } else -> callback.onError(apiError) } diff --git a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/librus/data/LibrusPortal.kt b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/librus/data/LibrusPortal.kt index 3c5e4963..b86a6956 100644 --- a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/librus/data/LibrusPortal.kt +++ b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/librus/data/LibrusPortal.kt @@ -44,7 +44,10 @@ open class LibrusPortal(open val data: DataLibrus) { "Access token is invalid" -> ERROR_LIBRUS_PORTAL_ACCESS_DENIED "ApiDisabled" -> ERROR_LIBRUS_PORTAL_API_DISABLED "Account not found" -> ERROR_LIBRUS_PORTAL_SYNERGIA_NOT_FOUND - else -> ERROR_LIBRUS_PORTAL_OTHER + else -> when (json.getString("hint")) { + "Error while decoding to JSON" -> ERROR_LIBRUS_PORTAL_ACCESS_DENIED + else -> ERROR_LIBRUS_PORTAL_OTHER + } }.let { errorCode -> data.error(ApiError(tag, errorCode) .withApiResponse(json) diff --git a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/models/Data.kt b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/models/Data.kt index 41e3e5b3..394aaeaf 100644 --- a/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/models/Data.kt +++ b/app/src/main/java/pl/szczodrzynski/edziennik/api/v2/models/Data.kt @@ -390,8 +390,6 @@ open class Data(val app: App, val profile: Profile?, val loginStore: LoginStore) } fun error(apiError: ApiError) { - if (apiError.isCritical) - cancel() callback.onError(apiError) } diff --git a/app/src/main/java/pl/szczodrzynski/edziennik/data/db/modules/login/LoginStore.java b/app/src/main/java/pl/szczodrzynski/edziennik/data/db/modules/login/LoginStore.java index f3d1b585..567f84db 100644 --- a/app/src/main/java/pl/szczodrzynski/edziennik/data/db/modules/login/LoginStore.java +++ b/app/src/main/java/pl/szczodrzynski/edziennik/data/db/modules/login/LoginStore.java @@ -8,6 +8,7 @@ import androidx.room.Entity; import androidx.room.Ignore; import com.google.gson.JsonElement; +import com.google.gson.JsonNull; import com.google.gson.JsonObject; import pl.szczodrzynski.edziennik.data.db.modules.profiles.ProfileFull; @@ -73,7 +74,7 @@ public class LoginStore { if (data == null) return defaultValue; JsonElement element = data.get(key); - if (element != null) { + if (element != null && !(element instanceof JsonNull)) { return element.getAsString(); } return defaultValue; @@ -83,7 +84,7 @@ public class LoginStore { if (data == null) return defaultValue; JsonElement element = data.get(key); - if (element != null) { + if (element != null && !(element instanceof JsonNull)) { return element.getAsInt(); } return defaultValue; @@ -93,7 +94,7 @@ public class LoginStore { if (data == null) return defaultValue; JsonElement element = data.get(key); - if (element != null) { + if (element != null && !(element instanceof JsonNull)) { return element.getAsLong(); } return defaultValue; @@ -103,7 +104,7 @@ public class LoginStore { if (data == null) return defaultValue; JsonElement element = data.get(key); - if (element != null) { + if (element != null && !(element instanceof JsonNull)) { return element.getAsFloat(); } return defaultValue; @@ -112,7 +113,7 @@ public class LoginStore { if (data == null) return defaultValue; JsonElement element = data.get(key); - if (element != null) { + if (element != null && !(element instanceof JsonNull)) { return element.getAsBoolean(); } return defaultValue;