From ad616472b3f0f178c50dfd22f3c30a9f28d471b0 Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Sun, 10 Aug 2025 13:35:29 -0500 Subject: [PATCH] Attempt to recover from HTTP 400 errors --- .../astrid/gtasks/api/GtasksInvoker.kt | 60 ++++++++----- .../tasks/gtasks/GoogleTaskSynchronizer.kt | 89 +++++++++++++++---- 2 files changed, 112 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/com/todoroo/astrid/gtasks/api/GtasksInvoker.kt b/app/src/main/java/com/todoroo/astrid/gtasks/api/GtasksInvoker.kt index 907c56c0b..dec40ec19 100644 --- a/app/src/main/java/com/todoroo/astrid/gtasks/api/GtasksInvoker.kt +++ b/app/src/main/java/com/todoroo/astrid/gtasks/api/GtasksInvoker.kt @@ -7,6 +7,7 @@ import com.google.api.services.tasks.model.Task import com.google.api.services.tasks.model.TaskList import com.google.api.services.tasks.model.TaskLists import org.tasks.googleapis.BaseInvoker +import timber.log.Timber import java.io.IOException /** @@ -43,21 +44,30 @@ class GtasksInvoker( @Throws(IOException::class) suspend fun getAllPositions( - listId: String?, pageToken: String?): com.google.api.services.tasks.model.Tasks? = - execute( - service!! - .tasks() - .list(listId) - .setMaxResults(100) - .setShowDeleted(false) - .setShowHidden(false) - .setPageToken(pageToken) - .setFields("items(id,parent,position),nextPageToken")) + listId: String?, + pageToken: String?, + ): com.google.api.services.tasks.model.Tasks? = + execute( + service!! + .tasks() + .list(listId) + .setMaxResults(100) + .setShowDeleted(false) + .setShowHidden(false) + .setPageToken(pageToken) + .setFields("items(id,parent,position),nextPageToken") + ) @Throws(IOException::class) suspend fun createGtask( - listId: String?, task: Task?, parent: String?, previous: String?): Task? = - execute(service!!.tasks().insert(listId, task).setParent(parent).setPrevious(previous)) + listId: String?, + task: Task?, + parent: String?, + previous: String?, + ): Task? { + Timber.d("createGtask(listId=$listId, task=, parent=$parent, previous=$previous)") + return execute(service!!.tasks().insert(listId, task).setParent(parent).setPrevious(previous)) + } @Throws(IOException::class) suspend fun updateGtask(listId: String?, task: Task) = @@ -65,19 +75,26 @@ class GtasksInvoker( @Throws(IOException::class) suspend fun moveGtask( - listId: String?, taskId: String?, parentId: String?, previousId: String?): Task? = - execute( - service!! - .tasks() - .move(listId, taskId) - .setParent(parentId) - .setPrevious(previousId)) + listId: String?, + taskId: String?, + parentId: String?, + previousId: String?, + ): Task? { + Timber.d("moveGtask(listId=$listId, taskId=$taskId, parentId=$parentId, previousId=$previousId)") + return execute( + service!! + .tasks() + .move(listId, taskId) + .setParent(parentId) + .setPrevious(previousId) + ) + } @Throws(IOException::class) suspend fun deleteGtaskList(listId: String?) { try { execute(service!!.tasklists().delete(listId)) - } catch (ignored: HttpNotFoundException) { + } catch (_: HttpNotFoundException) { } } @@ -91,9 +108,10 @@ class GtasksInvoker( @Throws(IOException::class) suspend fun deleteGtask(listId: String?, taskId: String?) { + Timber.d("deleteGtask(listId=$listId, taskId=$taskId)") try { execute(service!!.tasks().delete(listId, taskId)) - } catch (ignored: HttpNotFoundException) { + } catch (_: HttpNotFoundException) { } } } \ No newline at end of file diff --git a/app/src/main/java/org/tasks/gtasks/GoogleTaskSynchronizer.kt b/app/src/main/java/org/tasks/gtasks/GoogleTaskSynchronizer.kt index 6eb132723..c18711850 100644 --- a/app/src/main/java/org/tasks/gtasks/GoogleTaskSynchronizer.kt +++ b/app/src/main/java/org/tasks/gtasks/GoogleTaskSynchronizer.kt @@ -15,6 +15,7 @@ import com.todoroo.astrid.service.TaskCreator import com.todoroo.astrid.service.TaskCreator.Companion.getDefaultAlarms import com.todoroo.astrid.service.TaskDeleter import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.delay import org.tasks.LocalBroadcastManager import org.tasks.R import org.tasks.Strings.isNullOrEmpty @@ -125,7 +126,21 @@ class GoogleTaskSynchronizer @Inject constructor( preferences.setString(R.string.p_default_list, null) } } - pushLocalChanges(account, gtasksInvoker) + val failedTasks = mutableSetOf() + var retryTaskId = pushLocalChanges(account, gtasksInvoker) + + while (retryTaskId != null) { + if (failedTasks.contains(retryTaskId)) { + throw IOException("Invalid Task ID: $retryTaskId") + } + failedTasks.add(retryTaskId) + + Timber.d("Retrying push local changes due to stale task ID $retryTaskId (${failedTasks.size} total failed tasks)") + + delay(1000) + + retryTaskId = pushLocalChanges(account, gtasksInvoker) + } for (list in caldavDao.getCalendarsByAccount(account.uuid!!)) { if (isNullOrEmpty(list.uuid)) { firebase.reportException(RuntimeException("Empty remote id")) @@ -164,15 +179,19 @@ class GoogleTaskSynchronizer @Inject constructor( } @Throws(IOException::class) - private suspend fun pushLocalChanges(account: CaldavAccount, gtasksInvoker: GtasksInvoker) { + private suspend fun pushLocalChanges(account: CaldavAccount, gtasksInvoker: GtasksInvoker): Long? { val tasks = taskDao.getGoogleTasksToPush(account.uuid!!) for (task in tasks) { - pushTask(task, gtasksInvoker) + val staleTaskId = pushTask(task, gtasksInvoker) + if (staleTaskId != null) { + return staleTaskId + } } + return null } @Throws(IOException::class) - private suspend fun pushTask(task: org.tasks.data.entity.Task, gtasksInvoker: GtasksInvoker) { + private suspend fun pushTask(task: org.tasks.data.entity.Task, gtasksInvoker: GtasksInvoker): Long? { for (deleted in googleTaskDao.getDeletedByTaskId(task.id)) { deleted.remoteId?.let { try { @@ -186,7 +205,7 @@ class GoogleTaskSynchronizer @Inject constructor( } googleTaskDao.delete(deleted) } - val gtasksMetadata = googleTaskDao.getByTaskId(task.id) ?: return + val gtasksMetadata = googleTaskDao.getByTaskId(task.id) ?: return null val remoteModel = Task() var newlyCreated = false val remoteId: String? @@ -207,7 +226,7 @@ class GoogleTaskSynchronizer @Inject constructor( // creating a task which may end up being cancelled. Also don't sync new but already // deleted tasks if (newlyCreated && (isNullOrEmpty(task.title) || task.deletionDate > 0)) { - return + return null } // Update the remote model's changed properties @@ -230,10 +249,11 @@ class GoogleTaskSynchronizer @Inject constructor( val parent = task.parent val localParent = if (parent > 0) googleTaskDao.getRemoteId(parent) else null val previous = googleTaskDao.getPrevious( - listId!!, if (isNullOrEmpty(localParent)) 0 else parent, task.order ?: 0) + listId, if (isNullOrEmpty(localParent)) 0 else parent, task.order ?: 0) val created: Task? = try { gtasksInvoker.createGtask(listId, remoteModel, localParent, previous) } catch (e: HttpNotFoundException) { + Timber.e(e, "Failed to create task, retry without parent or order") gtasksInvoker.createGtask(listId, remoteModel, null, null) } if (created != null) { @@ -241,8 +261,10 @@ class GoogleTaskSynchronizer @Inject constructor( gtasksMetadata.remoteId = created.id gtasksMetadata.calendar = listId setOrderAndParent(gtasksMetadata, created, task) + Timber.d("Created new task: $gtasksMetadata") } else { - return + Timber.e("Empty response when creating task") + return null } } else { try { @@ -251,29 +273,64 @@ class GoogleTaskSynchronizer @Inject constructor( val parent = task.parent val localParent = if (parent > 0) googleTaskDao.getRemoteId(parent) else null val previous = googleTaskDao.getPrevious( - listId!!, + listId, if (localParent.isNullOrBlank()) 0 else parent, - task.order ?: 0) + task.order ?: 0, + ) gtasksInvoker - .moveGtask(listId, remoteModel.id, localParent, previous) - ?.let { setOrderAndParent(gtasksMetadata, it, task) } + .moveGtask( + listId = listId, + taskId = remoteModel.id, + parentId = localParent, + previousId = previous, + ) + ?.let { + setOrderAndParent( + googleTask = gtasksMetadata, + task = it, + local = task, + ) + } } catch (e: GoogleJsonResponseException) { if (e.statusCode == 400) { - Timber.e(e) + Timber.w("HTTP 400: clearing parent and order") + firebase.reportException(e) + taskDao.setParent(0L, listOf(task.id)) + taskDao.setOrder(task.id, 0L) + googleTaskDao.update(gtasksMetadata.copy(isMoved = false)) + return task.id } else { throw e } } } // TODO: don't updateGtask if it was only moved - gtasksInvoker.updateGtask(listId, remoteModel) - } catch (e: HttpNotFoundException) { + try { + gtasksInvoker.updateGtask(listId, remoteModel) + } catch (e: GoogleJsonResponseException) { + if (e.statusCode == 400 && e.details?.message == "Invalid task ID") { + Timber.w("HTTP 400: Invalid task ID for ${remoteModel.id}, clearing to recreate on next sync") + firebase.reportException(e) + googleTaskDao.update( + gtasksMetadata.copy( + remoteId = "", + isMoved = false, + ) + ) + return task.id + } else { + throw e + } + } + } catch (_: HttpNotFoundException) { + Timber.w("HTTP 404, deleting $gtasksMetadata") googleTaskDao.delete(gtasksMetadata) - return + return null } } gtasksMetadata.isMoved = false write(task, gtasksMetadata) + return null } @Throws(IOException::class)