From 0ee82bd5ce6471c385492e77f169c29d4e00ea74 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 22 Oct 2017 13:45:56 +0100 Subject: [PATCH] Better handling of network errors and improved borders --- CliClient/app/app-gui.js | 60 ++++++++++++-- CliClient/app/gui/FolderListWidget.js | 2 +- CliClient/app/gui/NoteMetadataWidget.js | 36 ++++++++ CliClient/locales/en_GB.po | 3 + CliClient/locales/fr_FR.po | 3 + CliClient/locales/joplin.pot | 3 + CliClient/package.json | 4 +- CliClient/publish.sh | 2 +- ReactNativeClient/lib/models/note.js | 14 +++- ReactNativeClient/lib/onedrive-api.js | 48 +++++------ ReactNativeClient/lib/shim-init-node.js | 104 +++++++++++++++++++----- 11 files changed, 223 insertions(+), 56 deletions(-) create mode 100644 CliClient/app/gui/NoteMetadataWidget.js diff --git a/CliClient/app/app-gui.js b/CliClient/app/app-gui.js index d350d26567..75383abc1d 100644 --- a/CliClient/app/app-gui.js +++ b/CliClient/app/app-gui.js @@ -21,6 +21,7 @@ const RootWidget = require('tkwidgets/RootWidget.js'); const WindowWidget = require('tkwidgets/WindowWidget.js'); const NoteWidget = require('./gui/NoteWidget.js'); +const NoteMetadataWidget = require('./gui/NoteMetadataWidget.js'); const FolderListWidget = require('./gui/FolderListWidget.js'); const NoteListWidget = require('./gui/NoteListWidget.js'); const StatusBarWidget = require('./gui/StatusBarWidget.js'); @@ -87,7 +88,10 @@ class AppGui { this.rootWidget_.name = 'root'; const folderList = new FolderListWidget(); - folderList.style = { borderBottomWidth: 1 }; + folderList.style = { + borderBottomWidth: 1, + borderRightWidth: 1, + }; folderList.name = 'folderList'; folderList.vStretch = true; folderList.on('currentItemChange', async () => { @@ -127,25 +131,49 @@ class AppGui { }); const noteText = new NoteWidget(); - noteText.vStretch = true; + noteText.hStretch = true; noteText.name = 'noteText'; - noteText.style = { borderBottomWidth: 1 }; + noteText.style = { + borderBottomWidth: 1, + borderLeftWidth: 1, + }; this.rootWidget_.connect(noteText, (state) => { return { noteId: state.selectedNoteId }; }); + const noteMetadata = new NoteMetadataWidget(); + noteMetadata.hStretch = true; + noteMetadata.name = 'noteMetadata'; + noteMetadata.style = { + borderBottomWidth: 1, + borderLeftWidth: 1, + borderRightWidth: 1, + }; + this.rootWidget_.connect(noteMetadata, (state) => { + return { noteId: state.selectedNoteId }; + }); + noteMetadata.hide(); + const consoleWidget = new ConsoleWidget(); consoleWidget.hStretch = true; + consoleWidget.style = { + borderBottomWidth: 1, + }; consoleWidget.hide(); const statusBar = new StatusBarWidget(); statusBar.hStretch = true; + const noteLayout = new VLayoutWidget(); + noteLayout.name = 'noteLayout'; + noteLayout.addChild(noteText, { type: 'stretch', factor: 1 }); + noteLayout.addChild(noteMetadata, { type: 'stretch', factor: 1 }); + const hLayout = new HLayoutWidget(); hLayout.name = 'hLayout'; hLayout.addChild(folderList, { type: 'stretch', factor: 1 }); hLayout.addChild(noteList, { type: 'stretch', factor: 1 }); - hLayout.addChild(noteText, { type: 'stretch', factor: 2 }); + hLayout.addChild(noteLayout, { type: 'stretch', factor: 2 }); const vLayout = new VLayoutWidget(); vLayout.name = 'vLayout'; @@ -191,6 +219,14 @@ class AppGui { canRunAlongOtherCommands: true, } + shortcuts['m'] = { + description: _('Toggle note metadata.'), + action: () => { + this.toggleNoteMetadata(); + }, + canRunAlongOtherCommands: true, + } + shortcuts[':'] = { description: _('Enter command line mode'), action: async () => { @@ -253,9 +289,7 @@ class AppGui { } showConsole(doShow = true) { - const consoleWidget = this.widget('console'); - consoleWidget.show(doShow); - this.widget('root').invalidate(); + this.widget('console').show(doShow); } hideConsole() { @@ -293,6 +327,18 @@ class AppGui { return this.widget('console').isMaximized__ === true; } + showNoteMetadata(show = true) { + this.widget('noteMetadata').show(show); + } + + hideNoteMetadata() { + this.showNoteMetadata(false); + } + + toggleNoteMetadata() { + this.showNoteMetadata(!this.widget('noteMetadata').shown); + } + widget(name) { if (name === 'root') return this.rootWidget_; return this.rootWidget_.childByName(name); diff --git a/CliClient/app/gui/FolderListWidget.js b/CliClient/app/gui/FolderListWidget.js index d0429bbe02..2d628c821b 100644 --- a/CliClient/app/gui/FolderListWidget.js +++ b/CliClient/app/gui/FolderListWidget.js @@ -10,7 +10,7 @@ class FolderListWidget extends ListWidget { this.updateIndexFromSelectedFolderId_ = false; this.itemRenderer = (item) => { - return item.title + ' ' + item.id; + return item.title; //+ ' ' + item.id; }; } diff --git a/CliClient/app/gui/NoteMetadataWidget.js b/CliClient/app/gui/NoteMetadataWidget.js new file mode 100644 index 0000000000..5d534d8cbf --- /dev/null +++ b/CliClient/app/gui/NoteMetadataWidget.js @@ -0,0 +1,36 @@ +const Note = require('lib/models/note.js').Note; +const TextWidget = require('tkwidgets/TextWidget.js'); + +class NoteMetadataWidget extends TextWidget { + + constructor() { + super(); + this.noteId_ = 0; + this.note_ = null; + } + + get noteId() { + return this.noteId_; + } + + set noteId(v) { + // If this is called it means either the note ID has changed OR + // the note content has changed, so we always set note_ to null + // so that it can be reloaded in onWillRender(). + this.noteId_ = v; + this.note_ = null; + this.invalidate(); + } + + async onWillRender() { + if (!this.visible) return; + + if (!this.note_ && this.noteId_) { + this.note_ = await Note.load(this.noteId_); + this.text = this.note_ ? await Note.serializeAllProps(this.note_) : ''; + } + } + +} + +module.exports = NoteMetadataWidget; \ No newline at end of file diff --git a/CliClient/locales/en_GB.po b/CliClient/locales/en_GB.po index 291e3a9eeb..80eda90aca 100644 --- a/CliClient/locales/en_GB.po +++ b/CliClient/locales/en_GB.po @@ -24,6 +24,9 @@ msgstr "" msgid "Toggle console between maximized/minimized/hidden/visible." msgstr "" +msgid "Toggle note metadata." +msgstr "" + msgid "Enter command line mode" msgstr "" diff --git a/CliClient/locales/fr_FR.po b/CliClient/locales/fr_FR.po index e0c885aecc..6597d031d8 100644 --- a/CliClient/locales/fr_FR.po +++ b/CliClient/locales/fr_FR.po @@ -26,6 +26,9 @@ msgstr "Tâches non-complétées et récentes" msgid "Toggle console between maximized/minimized/hidden/visible." msgstr "" +msgid "Toggle note metadata." +msgstr "" + msgid "Enter command line mode" msgstr "" diff --git a/CliClient/locales/joplin.pot b/CliClient/locales/joplin.pot index 291e3a9eeb..80eda90aca 100644 --- a/CliClient/locales/joplin.pot +++ b/CliClient/locales/joplin.pot @@ -24,6 +24,9 @@ msgstr "" msgid "Toggle console between maximized/minimized/hidden/visible." msgstr "" +msgid "Toggle note metadata." +msgstr "" + msgid "Enter command line mode" msgstr "" diff --git a/CliClient/package.json b/CliClient/package.json index 88ba6a0a1d..7ae5508bfd 100644 --- a/CliClient/package.json +++ b/CliClient/package.json @@ -4,7 +4,7 @@ "license": "MIT", "author": "Laurent Cozic", "bugs": { - "url": "https://github.com/laurent22/joplin/issues" + "url": "https://github.com/laurent22/joplin/issues" }, "repository": { "type": "git", @@ -18,7 +18,7 @@ ], "owner": "Laurent Cozic" }, - "version": "0.10.44", + "version": "0.10.47", "bin": { "joplin": "./main.js" }, diff --git a/CliClient/publish.sh b/CliClient/publish.sh index 98fa3d72e4..df10f8c10c 100755 --- a/CliClient/publish.sh +++ b/CliClient/publish.sh @@ -2,7 +2,7 @@ set -e SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -yarn upgrade +# yarn upgrade npm version patch #$SCRIPT_DIR/update-package-md5.sh touch "$SCRIPT_DIR/app/main.js" diff --git a/ReactNativeClient/lib/models/note.js b/ReactNativeClient/lib/models/note.js index 3f68436b76..ccea62f0ce 100644 --- a/ReactNativeClient/lib/models/note.js +++ b/ReactNativeClient/lib/models/note.js @@ -259,11 +259,21 @@ class Note extends BaseItem { geoData = Object.assign({}, this.geolocationCache_); } else { this.geolocationUpdating_ = true; + this.logger().info('Fetching geolocation...'); - geoData = await shim.Geolocation.currentPosition(); + try { + geoData = await shim.Geolocation.currentPosition(); + } catch (error) { + this.logger().error('Could not get lat/long for note ' + noteId + ': ', error); + geoData = null; + } + + this.geolocationUpdating_ = false; + + if (!geoData) return; + this.logger().info('Got lat/long'); this.geolocationCache_ = geoData; - this.geolocationUpdating_ = false; } this.logger().info('Updating lat/long of note ' + noteId); diff --git a/ReactNativeClient/lib/onedrive-api.js b/ReactNativeClient/lib/onedrive-api.js index 23ac597dcd..0c4c49de58 100644 --- a/ReactNativeClient/lib/onedrive-api.js +++ b/ReactNativeClient/lib/onedrive-api.js @@ -173,32 +173,32 @@ class OneDriveApi { response = await shim.fetchBlob(url, options); } } catch (error) { - let canRetry = true; + // let canRetry = true; - if (error.message == 'Network request failed') { - // Unfortunately the error 'Network request failed' doesn't have a type - // or error code, so hopefully that message won't change and is not localized - } else if (error.code == 'ECONNRESET') { - // request to https://public-ch3302....1fab24cb1bd5f.md failed, reason: socket hang up" - } else if (error.code == 'ENOTFOUND') { - // OneDrive (or Node?) sometimes sends back a "not found" error for resources - // that definitely exist and in this case repeating the request works. - // Error is: - // request to https://graph.microsoft.com/v1.0/drive/special/approot failed, reason: getaddrinfo ENOTFOUND graph.microsoft.com graph.microsoft.com:443 - } else if (error.message.indexOf('network timeout') === 0) { - // network timeout at: https://public-ch3302...859f9b0e3ab.md - } else { - canRetry = false; - } + // if (error.message == 'Network request failed') { + // // Unfortunately the error 'Network request failed' doesn't have a type + // // or error code, so hopefully that message won't change and is not localized + // } else if (error.code == 'ECONNRESET') { + // // request to https://public-ch3302....1fab24cb1bd5f.md failed, reason: socket hang up" + // } else if (error.code == 'ENOTFOUND') { + // // OneDrive (or Node?) sometimes sends back a "not found" error for resources + // // that definitely exist and in this case repeating the request works. + // // Error is: + // // request to https://graph.microsoft.com/v1.0/drive/special/approot failed, reason: getaddrinfo ENOTFOUND graph.microsoft.com graph.microsoft.com:443 + // } else if (error.message.indexOf('network timeout') === 0) { + // // network timeout at: https://public-ch3302...859f9b0e3ab.md + // } else { + // canRetry = false; + // } - if (canRetry) { - this.logger().info('Got error code ' + error.code + ': ' + error.message + ' - retrying (' + i + ')...'); - await time.sleep((i + 1) * 3); - continue; - } else { - this.logger().error('Got unhandled error:', error ? error.code : '', error ? error.message : '', error); - throw error; - } + // if (canRetry) { + // this.logger().info('Got error code ' + error.code + ': ' + error.message + ' - retrying (' + i + ')...'); + // await time.sleep((i + 1) * 3); + // continue; + // } else { + this.logger().error('Got unhandled error:', error ? error.code : '', error ? error.message : '', error); + throw error; + //} } if (!response.ok) { diff --git a/ReactNativeClient/lib/shim-init-node.js b/ReactNativeClient/lib/shim-init-node.js index 6ded2c665f..63d7a2c63e 100644 --- a/ReactNativeClient/lib/shim-init-node.js +++ b/ReactNativeClient/lib/shim-init-node.js @@ -2,7 +2,38 @@ import fs from 'fs-extra'; import { shim } from 'lib/shim.js'; import { GeolocationNode } from 'lib/geolocation-node.js'; import { FileApiDriverLocal } from 'lib/file-api-driver-local.js'; +import { time } from 'lib/time-utils.js'; +function fetchRequestCanBeRetried(error) { + if (!error) return false; + + // Unfortunately the error 'Network request failed' doesn't have a type + // or error code, so hopefully that message won't change and is not localized + if (error.message == 'Network request failed') return true; + + // request to https://public-ch3302....1fab24cb1bd5f.md failed, reason: socket hang up" + if (error.code == 'ECONNRESET') return true; + + // OneDrive (or Node?) sometimes sends back a "not found" error for resources + // that definitely exist and in this case repeating the request works. + // Error is: + // request to https://graph.microsoft.com/v1.0/drive/special/approot failed, reason: getaddrinfo ENOTFOUND graph.microsoft.com graph.microsoft.com:443 + if (error.code == 'ENOTFOUND') return true; + + // network timeout at: https://public-ch3302...859f9b0e3ab.md + if (error.message && error.message.indexOf('network timeout') === 0) return true; + + // name: 'FetchError', + // message: 'request to https://api.ipify.org/?format=json failed, reason: getaddrinfo EAI_AGAIN api.ipify.org:443', + // type: 'system', + // errno: 'EAI_AGAIN', + // code: 'EAI_AGAIN' } } reason: { FetchError: request to https://api.ipify.org/?format=json failed, reason: getaddrinfo EAI_AGAIN api.ipify.org:443 + // + // It's a Microsoft error: "A temporary failure in name resolution occurred." + if (error.code == 'EAI_AGAIN') return true; + + return false; +} function shimInit() { shim.fs = fs; @@ -12,15 +43,32 @@ function shimInit() { const nodeFetch = require('node-fetch'); - shim.fetch = function(url, options = null) { + shim.fetch = async function(url, options = null) { if (!options) options = {}; if (!options.timeout) options.timeout = 1000 * 120; // ms - return nodeFetch(url, options); + if (!('maxRetry' in options)) options.maxRetry = 5; + + let retryCount = 0; + while (true) { + try { + const response = await nodeFetch(url, options); + return response; + } catch (error) { + if (fetchRequestCanBeRetried(error)) { + retryCount++; + if (retryCount > options.maxRetry) throw error; + await time.sleep(retryCount * 3); + } else { + throw error; + } + } + } } shim.fetchBlob = async function(url, options) { if (!options || !options.path) throw new Error('fetchBlob: target file path is missing'); if (!options.method) options.method = 'GET'; + if (!('maxRetry' in options)) options.maxRetry = 5; const urlParse = require('url').parse; @@ -51,30 +99,48 @@ function shimInit() { headers: headers, }; - return new Promise((resolve, reject) => { - try { - // Note: relative paths aren't supported - const file = fs.createWriteStream(filePath); + const doFetchOperation = async () => { + return new Promise((resolve, reject) => { + try { + // Note: relative paths aren't supported + const file = fs.createWriteStream(filePath); - const request = http.get(requestOptions, function(response) { - response.pipe(file); + const request = http.get(requestOptions, function(response) { + response.pipe(file); - file.on('finish', function() { - file.close(() => { - resolve(makeResponse(response)); + file.on('finish', function() { + file.close(() => { + resolve(makeResponse(response)); + }); }); - }); - }) + }) - request.on('error', function(error) { + request.on('error', function(error) { + fs.unlink(filePath); + reject(error); + }); + } catch(error) { fs.unlink(filePath); reject(error); - }); - } catch(error) { - fs.unlink(filePath); - reject(error); + } + }); + }; + + let retryCount = 0; + while (true) { + try { + const response = await doFetchOperation(); + return response; + } catch (error) { + if (fetchRequestCanBeRetried(error)) { + retryCount++; + if (retryCount > options.maxRetry) throw error; + await time.sleep(retryCount * 3); + } else { + throw error; + } } - }); + } } }