From f74db06176e1addca177b27530b2e4692cecad14 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 28 Dec 2019 20:23:38 +0100 Subject: [PATCH] All: Better handling of resource download errors, and added resource info to sync status screen --- ElectronClient/app/gui/NoteText.jsx | 6 ++- ReactNativeClient/lib/models/Resource.js | 39 ++++++++++++++++- .../lib/services/ResourceFetcher.js | 3 ++ ReactNativeClient/lib/services/report.js | 42 +++++++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/ElectronClient/app/gui/NoteText.jsx b/ElectronClient/app/gui/NoteText.jsx index 5968f8c26b..12e99ebf15 100644 --- a/ElectronClient/app/gui/NoteText.jsx +++ b/ElectronClient/app/gui/NoteText.jsx @@ -843,7 +843,11 @@ class NoteTextComponent extends React.Component { if (item.type_ === BaseModel.TYPE_RESOURCE) { const localState = await Resource.localState(item); if (localState.fetch_status !== Resource.FETCH_STATUS_DONE || !!item.encryption_blob_encrypted) { - bridge().showErrorMessageBox(_('This attachment is not downloaded or not decrypted yet.')); + if (localState.fetch_status === Resource.FETCH_STATUS_ERROR) { + bridge().showErrorMessageBox(`${_('There was an error downloading this attachment:')}\n\n${localState.fetch_error}`); + } else { + bridge().showErrorMessageBox(_('This attachment is not downloaded or not decrypted yet')); + } return; } const filePath = Resource.fullPath(item); diff --git a/ReactNativeClient/lib/models/Resource.js b/ReactNativeClient/lib/models/Resource.js index 0fd0841fd0..38c5067909 100644 --- a/ReactNativeClient/lib/models/Resource.js +++ b/ReactNativeClient/lib/models/Resource.js @@ -9,6 +9,7 @@ const { filename, safeFilename } = require('lib/path-utils.js'); const { FsDriverDummy } = require('lib/fs-driver-dummy.js'); const markdownUtils = require('lib/markdownUtils'); const JoplinError = require('lib/JoplinError'); +const { _ } = require('lib/locale.js'); class Resource extends BaseItem { static tableName() { @@ -34,6 +35,15 @@ class Resource extends BaseItem { return this.db().selectAll(`SELECT resource_id, fetch_status FROM resource_local_states WHERE resource_id IN ("${resourceIds.join('","')}")`); } + static errorFetchStatuses() { + return this.db().selectAll(` + SELECT title AS resource_title, resource_id, fetch_error + FROM resource_local_states + LEFT JOIN resources ON resources.id = resource_local_states.resource_id + WHERE fetch_status = ? + `, [Resource.FETCH_STATUS_ERROR]); + } + static needToBeFetched(resourceDownloadMode = null, limit = null) { let sql = ['SELECT * FROM resources WHERE encryption_applied = 0 AND id IN (SELECT resource_id FROM resource_local_states WHERE fetch_status = ?)']; if (resourceDownloadMode !== 'always') { @@ -48,6 +58,10 @@ class Resource extends BaseItem { return await this.db().exec('UPDATE resource_local_states SET fetch_status = ? WHERE fetch_status = ?', [Resource.FETCH_STATUS_IDLE, Resource.FETCH_STATUS_STARTED]); } + static resetErrorStatus(resourceId) { + return this.db().exec('UPDATE resource_local_states SET fetch_status = ?, fetch_error = "" WHERE resource_id = ?', [Resource.FETCH_STATUS_IDLE, resourceId]); + } + static fsDriver() { if (!Resource.fsDriver_) Resource.fsDriver_ = new FsDriverDummy(); return Resource.fsDriver_; @@ -249,10 +263,33 @@ class Resource extends BaseItem { } static async downloadedButEncryptedBlobCount() { - const r = await this.db().selectOne('SELECT count(*) as total FROM resource_local_states WHERE fetch_status = ? AND resource_id IN (SELECT id FROM resources WHERE encryption_blob_encrypted = 1)', [Resource.FETCH_STATUS_DONE]); + const r = await this.db().selectOne(` + SELECT count(*) as total + FROM resource_local_states + WHERE fetch_status = ? AND resource_id IN (SELECT id FROM resources WHERE encryption_blob_encrypted = 1) + `, [Resource.FETCH_STATUS_DONE]); return r ? r.total : 0; } + + static async downloadStatusCounts(status) { + const r = await this.db().selectOne(` + SELECT count(*) as total + FROM resource_local_states + WHERE fetch_status = ? + `, [status]); + + return r ? r.total : 0; + } + + static fetchStatusToLabel(status) { + if (status === Resource.FETCH_STATUS_IDLE) return _('Not downloaded'); + if (status === Resource.FETCH_STATUS_STARTED) return _('Downloading'); + if (status === Resource.FETCH_STATUS_DONE) return _('Downloaded'); + if (status === Resource.FETCH_STATUS_ERROR) return _('Error'); + throw new Error(`Invalid status: ${status}`); + } + } Resource.IMAGE_MAX_DIMENSION = 1920; diff --git a/ReactNativeClient/lib/services/ResourceFetcher.js b/ReactNativeClient/lib/services/ResourceFetcher.js index 2ad73e3412..d7f0dadd8f 100644 --- a/ReactNativeClient/lib/services/ResourceFetcher.js +++ b/ReactNativeClient/lib/services/ResourceFetcher.js @@ -222,6 +222,9 @@ class ResourceFetcher extends BaseService { this.logger().info(`ResourceFetcher: Auto-added resources: ${count}`); this.addingResources_ = false; + + const errorCount = await Resource.downloadStatusCounts(Resource.FETCH_STATUS_ERROR); + if (errorCount) this.dispatch({ type: 'SYNC_HAS_DISABLED_SYNC_ITEMS' }); } async start() { diff --git a/ReactNativeClient/lib/services/report.js b/ReactNativeClient/lib/services/report.js index aa067a4a54..5501bd1676 100644 --- a/ReactNativeClient/lib/services/report.js +++ b/ReactNativeClient/lib/services/report.js @@ -5,6 +5,8 @@ const Folder = require('lib/models/Folder.js'); const Note = require('lib/models/Note.js'); const BaseModel = require('lib/BaseModel.js'); const DecryptionWorker = require('lib/services/DecryptionWorker'); +const ResourceFetcher = require('lib/services/ResourceFetcher'); +const Resource = require('lib/models/Resource'); const { _ } = require('lib/locale.js'); const { toTitleCase } = require('lib/string-utils.js'); @@ -158,6 +160,46 @@ class ReportService { sections.push(section); } + { + section = { title: _('Attachments'), body: [], name: 'resources' }; + + const statuses = [Resource.FETCH_STATUS_IDLE, Resource.FETCH_STATUS_STARTED, Resource.FETCH_STATUS_DONE, Resource.FETCH_STATUS_ERROR]; + + for (const status of statuses) { + if (status === Resource.FETCH_STATUS_DONE) { + const downloadedButEncryptedBlobCount = await Resource.downloadedButEncryptedBlobCount(); + const downloadedCount = await Resource.downloadStatusCounts(Resource.FETCH_STATUS_DONE); + section.body.push(_('%s: %d', _('Downloaded and decrypted'), downloadedCount - downloadedButEncryptedBlobCount)); + section.body.push(_('%s: %d', _('Downloaded and encrypted'), downloadedButEncryptedBlobCount)); + } else { + const count = await Resource.downloadStatusCounts(status); + section.body.push(_('%s: %d', Resource.fetchStatusToLabel(status), count)); + } + } + + sections.push(section); + } + + const resourceErrorFetchStatuses = await Resource.errorFetchStatuses(); + + if (resourceErrorFetchStatuses.length) { + section = { title: _('Attachments that could not be downloaded'), body: [], name: 'failedResourceDownload' }; + + for (let i = 0; i < resourceErrorFetchStatuses.length; i++) { + const row = resourceErrorFetchStatuses[i]; + section.body.push({ + text: _('%s (%s): %s', row.resource_title, row.resource_id, row.fetch_error), + canRetry: true, + retryHandler: async () => { + await Resource.resetErrorStatus(row.resource_id); + ResourceFetcher.instance().autoAddResources(); + }, + }); + } + + sections.push(section); + } + section = { title: _('Sync status (synced items / total items)'), body: [] }; for (let n in r.items) {