Better error handling during sync setup

CliClient/.gitignore

@@ -10,4 +10,5 @@ tests/fuzzing/client1
 tests/fuzzing/client2
 tests/fuzzing/sync
 tests/fuzzing.*
-tests/fuzzing -*
+tests/fuzzing -*
+tests/logs/*

CliClient/app/main.js

@@ -46,6 +46,7 @@ Logger.fsDriver_ = fsDriver;
 Resource.fsDriver_ = fsDriver;
 
 Setting.setConstant('appId', 'net.cozic.joplin-cli');
+Setting.setConstant('appType', 'cli');
 
 let currentFolder = null;
 let commands = [];
@@ -509,7 +510,7 @@ commands.push({
 	options: [
 		['--random-failures', 'For debugging purposes. Do not use.'],
 	],
-	action: function(args, end) {
+	action: async function(args, end) {
 
 		let options = {
 			onProgress: (report) => {
@@ -528,16 +529,23 @@ commands.push({
 		};
 
 		this.log(_('Synchronization target: %s', Setting.value('sync.target')));
-		synchronizer(Setting.value('sync.target')).then((s) => {
-			this.log(_('Starting synchronization...'));
-			return s.start(options);
-		}).catch((error) => {
-			this.log(error);
-		}).then(() => {
-			vorpalUtils.redrawDone();
-			this.log(_('Done.'));
+
+		let sync = await synchronizer(Setting.value('sync.target'));
+		if (!sync) {
 			end();
-		});
+			return;
+		}
+
+		try {
+			this.log(_('Starting synchronization...'));
+			await sync.start(options);
+		} catch (error) {
+			this.log(error);
+		}
+
+		vorpalUtils.redrawDone();
+		this.log(_('Done.'));
+		end();
 	},
 });
 
@@ -682,13 +690,12 @@ async function synchronizer(syncTarget) {
 		const oneDriveApi = reg.oneDriveApi();
 		let driver = new FileApiDriverOneDrive(oneDriveApi);
 		let auth = Setting.value('sync.onedrive.auth');
-		
-		if (auth) {
-			auth = JSON.parse(auth);
-		} else {
+
+		if (!oneDriveApi.auth()) {
 			const oneDriveApiUtils = new OneDriveApiNodeUtils(oneDriveApi);
 			auth = await oneDriveApiUtils.oauthDance(vorpal);
-			Setting.setValue('sync.onedrive.auth', JSON.stringify(auth));
+			Setting.setValue('sync.onedrive.auth', auth ? JSON.stringify(auth) : auth);
+			if (!auth) return;
 		}
 
 		let appDir = await oneDriveApi.appDirectory();
@@ -709,7 +716,7 @@ async function synchronizer(syncTarget) {
 		throw new Error('Unknown backend: ' + syncTarget);
 	}
 
-	synchronizers_[syncTarget] = new Synchronizer(database_, fileApi);
+	synchronizers_[syncTarget] = new Synchronizer(database_, fileApi, Setting.value('appType'));
 	synchronizers_[syncTarget].setLogger(syncLogger);
 
 	return synchronizers_[syncTarget];

CliClient/app/onedrive-api-node-utils.js

@@ -53,39 +53,6 @@ class OneDriveApiNodeUtils {
 
 				if (!query.code) return writeResponse(400, '"code" query parameter is missing');
 
-				// let body = new FormData();
-				// body.append('client_id', this.api().clientId());
-				// body.append('client_secret', this.api().clientSecret());
-				// body.append('code', query.code ? query.code : '');
-				// body.append('redirect_uri', 'http://localhost:' + port.toString());
-				// body.append('grant_type', 'authorization_code');
-
-				// let options = {
-				// 	method: 'POST',
-				// 	body: body,
-				// };
-
-				// fetch(this.api().tokenBaseUrl(), options).then((r) => {
-
-				// this.api().execTokenRequest(query.code, 'http://localhost:' + port.toString()).then((r) => {
-				// 	if (!r.ok) {
-				// 		errorMessage = 'Could not retrieve auth code: ' + r.status + ': ' + r.statusText;
-				// 		writeResponse(400, errorMessage);
-				// 		targetConsole.log('');
-				// 		targetConsole.log(errorMessage);
-				// 		server.destroy();
-				// 		return;
-				// 	}
-
-				// 	return r.json().then((json) => {
-				// 		this.api().setAuth(json);
-				// 		writeResponse(200, 'The application has been authorised - you may now close this browser tab.');
-				// 		targetConsole.log('');
-				// 		targetConsole.log('The application has been successfully authorised.');
-				// 		server.destroy();
-				// 	});
-				// });
-
 				this.api().execTokenRequest(query.code, 'http://localhost:' + port.toString()).then(() => {
 					writeResponse(200, 'The application has been authorised - you may now close this browser tab.');
 					targetConsole.log('');

CliClient/tests/test-utils.js

@@ -35,6 +35,9 @@ BaseItem.loadClass('Resource', Resource);
 BaseItem.loadClass('Tag', Tag);
 BaseItem.loadClass('NoteTag', NoteTag);
 
+Setting.setConstant('appId', 'net.cozic.joplin-cli');
+Setting.setConstant('appType', 'cli');
+
 function sleep(n) {
 	return new Promise((resolve, reject) => {
 		setTimeout(() => {
@@ -99,7 +102,7 @@ async function setupDatabaseAndSynchronizer(id = null) {
 	await setupDatabase(id);
 
 	if (!synchronizers_[id]) {
-		synchronizers_[id] = new Synchronizer(db(id), fileApi());
+		synchronizers_[id] = new Synchronizer(db(id), fileApi(), Setting.value('appType'));
 		synchronizers_[id].setLogger(logger);
 	}
 

ReactNativeClient/lib/models/setting.js

@@ -69,7 +69,11 @@ class Setting extends BaseModel {
 	}
 
 	static value(key) {
-		if (key in this.constants_) return this.constants_[key];
+		if (key in this.constants_) {
+			let output = this.constants_[key];
+			if (output == 'SET_ME') throw new Error('Setting constant has not been set: ' + key);
+			return output;
+		}
 
 		if (!this.cache_) throw new Error('Settings have not been initialized!');
 
@@ -153,6 +157,7 @@ Setting.defaults_ = {
 Setting.constants_ = {
 	'appName': 'joplin',
 	'appId': 'SET_ME', // Each app should set this identifier
+	'appType': 'SET_ME', // 'cli' or 'mobile'
 	'resourceDir': '',
 	'profileDir': '',
 	'tempDir': '',

ReactNativeClient/lib/onedrive-api.js

@@ -4,15 +4,24 @@ import { time } from 'lib/time-utils.js';
 
 class OneDriveApi {
 
-	constructor(clientId, clientSecret) {
+	// `isPublic` is to tell OneDrive whether the application is a "public" one (Mobile and desktop
+	// apps are considered "public"), in which case the secret should not be sent to the API.
+	// In practice the React Native app is public, and the Node one is not because we
+	// use a local server for the OAuth dance.
+	constructor(clientId, clientSecret, isPublic) {
 		this.clientId_ = clientId;
 		this.clientSecret_ = clientSecret;
 		this.auth_ = null;
+		this.isPublic_ = isPublic;
 		this.listeners_ = {
 			'authRefreshed': [],
 		};
 	}
 
+	isPublic() {
+		return this.isPublic_;
+	}
+
 	dispatch(eventName, param) {
 		let ls = this.listeners_[eventName];
 		for (let i = 0; i < ls.length; i++) {
@@ -34,6 +43,7 @@ class OneDriveApi {
 
 	setAuth(auth) {
 		this.auth_ = auth;
+		this.dispatch('authRefreshed', this.auth());
 	}
 
 	token() {
@@ -63,10 +73,10 @@ class OneDriveApi {
 		return 'https://login.microsoftonline.com/common/oauth2/v2.0/authorize?' + stringify(query);
 	}
 
-	async execTokenRequest(code, redirectUri, isPublic = false) {
+	async execTokenRequest(code, redirectUri) {
 		let body = new shim.FormData();
 		body.append('client_id', this.clientId());
-		if (!isPublic) body.append('client_secret', this.clientSecret());
+		if (!this.isPublic()) body.append('client_secret', this.clientSecret());
 		body.append('code', code);
 		body.append('redirect_uri', redirectUri);
 		body.append('grant_type', 'authorization_code');
@@ -84,8 +94,8 @@ class OneDriveApi {
 		try {
 			const json = await r.json();
 			this.setAuth(json);
-			this.dispatch('authRefreshed', this.auth());
 		} catch (error) {
+			this.setAuth(null);
 			const text = await r.text();
 			error.message += ': ' + text;
 			throw error;
@@ -202,7 +212,7 @@ class OneDriveApi {
 
 		let body = new shim.FormData();
 		body.append('client_id', this.clientId());
-		// body.append('client_secret', this.clientSecret()); // TODO: NEEDED FOR NODE
+		if (!this.isPublic()) body.append('client_secret', this.clientSecret());
 		body.append('refresh_token', this.auth_.refresh_token);
 		body.append('redirect_uri', 'http://localhost:1917');
 		body.append('grant_type', 'refresh_token');
@@ -212,17 +222,15 @@ class OneDriveApi {
 			body: body,
 		};
 
-		this.auth_ = null;
-
 		let response = await shim.fetch(this.tokenBaseUrl(), options);
 		if (!response.ok) {
+			this.setAuth(null);
 			let msg = await response.text();
 			throw new Error(msg);
 		}
 
-		this.auth_ = await response.json();
-
-		this.dispatch('authRefreshed', this.auth_);
+		let auth = await response.json();
+		this.setAuth(auth);
 	}
 
 }

ReactNativeClient/lib/registry.js

@@ -20,26 +20,31 @@ reg.oneDriveApi = () => {
 
 	const CLIENT_ID = 'e09fc0de-c958-424f-83a2-e56a721d331b';
 	const CLIENT_SECRET = 'JA3cwsqSGHFtjMwd5XoF5L5';
-	reg.oneDriveApi_ = new OneDriveApi(CLIENT_ID, CLIENT_SECRET);
+	const isPublic = Setting.value('appType') != 'cli';
 
-	let auth = Setting.value('sync.onedrive.auth');
-	if (auth) {
-		auth = JSON.parse(auth);
-		reg.oneDriveApi_.setAuth(auth);
-	}
+	reg.oneDriveApi_ = new OneDriveApi(CLIENT_ID, CLIENT_SECRET, isPublic);
 
 	reg.oneDriveApi_.on('authRefreshed', (a) => {
 		reg.logger().info('Saving updated OneDrive auth.');
-		Setting.setValue('sync.onedrive.auth', JSON.stringify(a));
+		Setting.setValue('sync.onedrive.auth', a ? JSON.stringify(a) : null);
 	});
+
+	let auth = Setting.value('sync.onedrive.auth');
+	if (auth) {
+		try {
+			auth = JSON.parse(auth);
+		} catch (error) {
+			reg.logger().warn('Could not parse OneDrive auth token');
+			reg.logger().warn(error);
+			auth = null;
+		}
+
+		reg.oneDriveApi_.setAuth(auth);
+	}
 	
 	return reg.oneDriveApi_;
 }
 
-reg.setFileApi = (v) => {
-	reg.fileApi_ = v;
-}
-
 reg.fileApi = async () => {
 	if (reg.fileApi_) return reg.fileApi_;
 
@@ -58,7 +63,7 @@ reg.synchronizer = async () => {
 	if (!reg.db()) throw new Error('Cannot initialize synchronizer: db not initialized');
 
 	let fileApi = await reg.fileApi();
-	reg.synchronizer_ = new Synchronizer(reg.db(), fileApi);
+	reg.synchronizer_ = new Synchronizer(reg.db(), fileApi, Setting.value('appType'));
 	reg.synchronizer_.setLogger(reg.logger());
 	return reg.synchronizer_;
 }

ReactNativeClient/lib/synchronizer.js

@@ -10,13 +10,14 @@ import moment from 'moment';
 
 class Synchronizer {
 
-	constructor(db, api) {
+	constructor(db, api, appType) {
 		this.state_ = 'idle';
 		this.db_ = db;
 		this.api_ = api;
 		this.syncDirName_ = '.sync';
 		this.resourceDirName_ = '.resource';
 		this.logger_ = new Logger();
+		this.appType_ = appType;
 	}
 
 	state() {
@@ -313,17 +314,17 @@ class Synchronizer {
 						};
 						if (action == 'createLocal') options.isNew = true;
 
-						// if (newContent.type_ == BaseModel.TYPE_RESOURCE && action == 'createLocal') {
-						// 	let localResourceContentPath = Resource.fullPath(newContent);
-						// 	let remoteResourceContentPath = this.resourceDirName_ + '/' + newContent.id;
-						// 	let remoteResourceContent = await this.api().get(remoteResourceContentPath, { encoding: 'binary' });
-						// 	await Resource.setContent(newContent, remoteResourceContent);
-						// }
-
 						if (newContent.type_ == BaseModel.TYPE_RESOURCE && action == 'createLocal') {
 							let localResourceContentPath = Resource.fullPath(newContent);
 							let remoteResourceContentPath = this.resourceDirName_ + '/' + newContent.id;
-							await this.api().get(remoteResourceContentPath, { path: localResourceContentPath, target: 'file' });
+							if (this.appType_ == 'cli') {								
+								let remoteResourceContent = await this.api().get(remoteResourceContentPath, { encoding: 'binary' });
+								await Resource.setContent(newContent, remoteResourceContent);
+							} else if (this.appType_ == 'mobile') {
+								await this.api().get(remoteResourceContentPath, { path: localResourceContentPath, target: 'file' });
+							} else {
+								throw new Error('Unknown appType: ' + this.appType_);
+							}
 						}
 
 						await ItemClass.save(newContent, options);

ReactNativeClient/root.js

@@ -263,6 +263,7 @@ class AppComponent extends React.Component {
 			await Setting.load();
 			
 			Setting.setConstant('appId', 'net.cozic.joplin-android');
+			Setting.setConstant('appType', 'mobile');
 			Setting.setConstant('resourceDir', RNFetchBlob.fs.dirs.DocumentDir);
 
 			Log.info('Loading folders...');