Skip to content

Commit

Permalink
Desktop: Disable initial Sentry request when crash auto-upload is dis…
Browse files Browse the repository at this point in the history
…abled
  • Loading branch information
laurent22 committed Feb 7, 2024
1 parent 01ec640 commit 07aba91
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 49 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Expand Up @@ -679,6 +679,7 @@ packages/lib/components/shared/side-menu-shared.js
packages/lib/database-driver-better-sqlite.js
packages/lib/database.js
packages/lib/debug/DebugService.js
packages/lib/determineProfileDir.js
packages/lib/dom.js
packages/lib/errorUtils.js
packages/lib/errors.js
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -659,6 +659,7 @@ packages/lib/components/shared/side-menu-shared.js
packages/lib/database-driver-better-sqlite.js
packages/lib/database.js
packages/lib/debug/DebugService.js
packages/lib/determineProfileDir.js
packages/lib/dom.js
packages/lib/errorUtils.js
packages/lib/errors.js
Expand Down
12 changes: 3 additions & 9 deletions packages/app-desktop/app.ts
Expand Up @@ -11,7 +11,7 @@ import AlarmServiceDriverNode from '@joplin/lib/services/AlarmServiceDriverNode'
import Logger, { TargetType } from '@joplin/utils/Logger';
import Setting from '@joplin/lib/models/Setting';
import actionApi from '@joplin/lib/services/rest/actionApi.desktop';
import BaseApplication from '@joplin/lib/BaseApplication';
import BaseApplication, { StartOptions } from '@joplin/lib/BaseApplication';
import DebugService from '@joplin/lib/debug/DebugService';
import { _, setLocale } from '@joplin/lib/locale';
import SpellCheckerService from '@joplin/lib/services/spellChecker/SpellCheckerService';
Expand Down Expand Up @@ -129,10 +129,6 @@ class Application extends BaseApplication {
this.setupOcrService();
}

if (action.type === 'SETTING_UPDATE_ONE' && action.key === 'autoUploadCrashDumps') {
bridge().setAutoUploadCrashDumps(action.value);
}

if (action.type === 'SETTING_UPDATE_ONE' && action.key === 'style.editor.fontFamily' || action.type === 'SETTING_UPDATE_ALL') {
this.updateEditorFont();
}
Expand Down Expand Up @@ -383,14 +379,12 @@ class Application extends BaseApplication {
eventManager.on(EventName.ResourceChange, handleResourceChange);
}

public async start(argv: string[]): Promise<any> {
public async start(argv: string[], startOptions: StartOptions = null): Promise<any> {
// If running inside a package, the command line, instead of being "node.exe <path> <flags>" is "joplin.exe <flags>" so
// insert an extra argument so that they can be processed in a consistent way everywhere.
if (!bridge().electronIsDev()) argv.splice(1, 0, '.');

argv = await super.start(argv);

bridge().setAutoUploadCrashDumps(Setting.value('autoUploadCrashDumps'));
argv = await super.start(argv, startOptions);

await this.applySettingsSideEffects();

Expand Down
45 changes: 35 additions & 10 deletions packages/app-desktop/bridge.ts
Expand Up @@ -25,16 +25,26 @@ export class Bridge {
private electronWrapper_: ElectronAppWrapper;
private lastSelectedPaths_: LastSelectedPath;
private autoUploadCrashDumps_ = false;
private rootProfileDir_: string;
private appName_: string;
private appId_: string;

public constructor(electronWrapper: ElectronAppWrapper) {
public constructor(electronWrapper: ElectronAppWrapper, appId: string, appName: string, rootProfileDir: string, autoUploadCrashDumps: boolean) {
this.electronWrapper_ = electronWrapper;
this.appId_ = appId;
this.appName_ = appName;
this.rootProfileDir_ = rootProfileDir;
this.autoUploadCrashDumps_ = autoUploadCrashDumps;
this.lastSelectedPaths_ = {
file: null,
directory: null,
};

Sentry.init({
dsn: 'https://cceec550871b1e8a10fee4c7a28d5cf2@o4506576757522432.ingest.sentry.io/4506594281783296',
this.sentryInit();
}

private sentryInit() {
const options: Sentry.ElectronMainOptions = {
beforeSend: event => {
try {
const date = (new Date()).toISOString().replace(/[:-]/g, '').split('.')[0];
Expand All @@ -49,7 +59,26 @@ export class Bridge {
return event;
}
},
});
};

if (this.autoUploadCrashDumps_) options.dsn = 'https://cceec550871b1e8a10fee4c7a28d5cf2@o4506576757522432.ingest.sentry.io/4506594281783296';

// eslint-disable-next-line no-console
console.info('Sentry: Initialized with autoUploadCrashDumps:', this.autoUploadCrashDumps_);

Sentry.init(options);
}

public appId() {
return this.appId_;
}

public appName() {
return this.appName_;
}

public rootProfileDir() {
return this.rootProfileDir_;
}

public electronApp() {
Expand All @@ -67,10 +96,6 @@ export class Bridge {
await msleep(10);
}

public setAutoUploadCrashDumps(v: boolean) {
this.autoUploadCrashDumps_ = v;
}

// The build directory contains additional external files that are going to
// be packaged by Electron Builder. This is for files that need to be
// accessed outside of the Electron app (for example the application icon).
Expand Down Expand Up @@ -328,9 +353,9 @@ export class Bridge {

let bridge_: Bridge = null;

export function initBridge(wrapper: ElectronAppWrapper) {
export function initBridge(wrapper: ElectronAppWrapper, appId: string, appName: string, rootProfileDir: string, autoUploadCrashDumps: boolean) {
if (bridge_) throw new Error('Bridge already initialized');
bridge_ = new Bridge(wrapper);
bridge_ = new Bridge(wrapper, appId, appName, rootProfileDir, autoUploadCrashDumps);
return bridge_;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/app-desktop/main-html.js
Expand Up @@ -80,7 +80,7 @@ const main = async () => {
BaseItem.loadClass('MasterKey', MasterKey);
BaseItem.loadClass('Revision', Revision);

Setting.setConstant('appId', `net.cozic.joplin${bridge().env() === 'dev' ? 'dev' : ''}-desktop`);
Setting.setConstant('appId', bridge().appId());
Setting.setConstant('appType', 'desktop');

// eslint-disable-next-line no-console
Expand Down
29 changes: 25 additions & 4 deletions packages/app-desktop/main.js
Expand Up @@ -3,12 +3,14 @@
const electronApp = require('electron').app;
require('@electron/remote/main').initialize();
const ElectronAppWrapper = require('./ElectronAppWrapper').default;
const { pathExistsSync, readFileSync } = require('fs-extra');
const { initBridge } = require('./bridge');
const Logger = require('@joplin/utils/Logger').default;
const FsDriverNode = require('@joplin/lib/fs-driver-node').default;
const envFromArgs = require('@joplin/lib/envFromArgs');
const packageInfo = require('./packageInfo.js');
const { isCallbackUrl } = require('@joplin/lib/callbackUrlUtils');
const determineProfileDir = require('@joplin/lib/determineProfileDir').default;

// Electron takes the application name from package.json `name` and
// displays this in the tray icon toolip and message box titles, however in
Expand All @@ -24,7 +26,7 @@ process.on('unhandledRejection', (reason, p) => {

// Likewise, we want to know if a profile is specified early, in particular
// to save the window state data.
function profileFromArgs(args) {
function getProfileFromArgs(args) {
if (!args) return null;
const profileIndex = args.indexOf('--profile');
if (profileIndex <= 0 || profileIndex >= args.length - 1) return null;
Expand All @@ -35,16 +37,35 @@ function profileFromArgs(args) {
Logger.fsDriver_ = new FsDriverNode();

const env = envFromArgs(process.argv);
const profilePath = profileFromArgs(process.argv);
const profileFromArgs = getProfileFromArgs(process.argv);
const isDebugMode = !!process.argv && process.argv.indexOf('--debug') >= 0;

// We initialize all these variables here because they are needed from the main process. They are
// then passed to the renderer process via the bridge.
const appId = `net.cozic.joplin${env === 'dev' ? 'dev' : ''}-desktop`;
let appName = env === 'dev' ? 'joplindev' : 'joplin';
if (appId.indexOf('-desktop') >= 0) appName += '-desktop';
const rootProfileDir = determineProfileDir(profileFromArgs, appName);
const settingsPath = `${rootProfileDir}/settings.json`;
let autoUploadCrashDumps = false;

if (pathExistsSync(settingsPath)) {
const settingsContent = readFileSync(settingsPath, 'utf8');
try {
const settings = JSON.parse(settingsContent);
autoUploadCrashDumps = !!settings && !!settings.autoUploadCrashDumps;
} catch (error) {
console.error(`Could not load settings: ${settingsPath}:`, error);
}
}

electronApp.setAsDefaultProtocolClient('joplin');

const initialCallbackUrl = process.argv.find((arg) => isCallbackUrl(arg));

const wrapper = new ElectronAppWrapper(electronApp, env, profilePath, isDebugMode, initialCallbackUrl);
const wrapper = new ElectronAppWrapper(electronApp, env, rootProfileDir, isDebugMode, initialCallbackUrl);

initBridge(wrapper);
initBridge(wrapper, appId, appName, rootProfileDir, autoUploadCrashDumps);

wrapper.start().catch((error) => {
console.error('Electron App fatal error:');
Expand Down
7 changes: 4 additions & 3 deletions packages/app-desktop/utils/restartInSafeModeFromMain.ts
@@ -1,16 +1,17 @@
import Setting from '@joplin/lib/models/Setting';
import bridge from '../bridge';
import processStartFlags from '@joplin/lib/utils/processStartFlags';
import BaseApplication, { safeModeFlagFilename } from '@joplin/lib/BaseApplication';
import { safeModeFlagFilename } from '@joplin/lib/BaseApplication';
import initProfile from '@joplin/lib/services/profileConfig/initProfile';
import { writeFile } from 'fs-extra';
import { join } from 'path';
import determineProfileDir from '@joplin/lib/determineProfileDir';


const restartInSafeModeFromMain = async () => {
// Only set constants here -- the main process doesn't have easy access (without loading
// a large amount of other code) to the database.
const appName = `joplin${bridge().env() === 'dev' ? 'dev' : ''}-desktop`;
const appName = bridge().appName();

This comment has been minimized.

Copy link
@personalizedrefrigerator

personalizedrefrigerator Feb 8, 2024

Collaborator

It looks like a test related to this line is now failing:

 FAIL utils/restartInSafeModeFromMain.test.js
 YN0000: [@joplin/app-desktop]:   ● restartInSafeModeFromMain › should create a safe mode flag file
 YN0000: [@joplin/app-desktop]: 
 YN0000: [@joplin/app-desktop]:     TypeError: (0 , bridge_1.default)(...).appName is not a function
 YN0000: [@joplin/app-desktop]: 
 YN0000: [@joplin/app-desktop]:       12 | 	// Only set constants here -- the main process doesn't have easy access (without loading
 YN0000: [@joplin/app-desktop]:       13 | 	// a large amount of other code) to the database.
 YN0000: [@joplin/app-desktop]:     > 14 | 	const appName = bridge().appName();
 YN0000: [@joplin/app-desktop]:          | 	                         ^
 YN0000: [@joplin/app-desktop]:       15 | 	Setting.setConstant('appId', `net.cozic.${appName}`);
 YN0000: [@joplin/app-desktop]:       16 | 	Setting.setConstant('appType', 'desktop');
 YN0000: [@joplin/app-desktop]:       17 | 	Setting.setConstant('appName', appName);
 YN0000: [@joplin/app-desktop]: 
 YN0000: [@joplin/app-desktop]:       at appName (utils/restartInSafeModeFromMain.ts:14:27)
 YN0000: [@joplin/app-desktop]:       at utils/restartInSafeModeFromMain.js:27:67
 YN0000: [@joplin/app-desktop]:       at Object.<anonymous>.__awaiter (utils/restartInSafeModeFromMain.js:9:10)
 YN0000: [@joplin/app-desktop]:       at __awaiter (utils/restartInSafeModeFromMain.ts:11:46)
 YN0000: [@joplin/app-desktop]:       at utils/restartInSafeModeFromMain.test.ts:38:34
 YN0000: [@joplin/app-desktop]:       at utils/restartInSafeModeFromMain.test.js:27:67
 YN0000: [@joplin/app-desktop]:       at Object.<anonymous>.__awaiter (utils/restartInSafeModeFromMain.test.js:9:10)
 YN0000: [@joplin/app-desktop]:       at Object.__awaiter (utils/restartInSafeModeFromMain.test.ts:37:57

It seems that the issue is caused by a partial mock of bridge.ts that's used for the test.

This comment has been minimized.

Copy link
@personalizedrefrigerator

personalizedrefrigerator Feb 8, 2024

Collaborator

Pull request: #9880

Setting.setConstant('appId', `net.cozic.${appName}`);
Setting.setConstant('appType', 'desktop');
Setting.setConstant('appName', appName);
Expand All @@ -20,7 +21,7 @@ const restartInSafeModeFromMain = async () => {
shimInit({});

const startFlags = await processStartFlags(bridge().processArgv());
const rootProfileDir = BaseApplication.determineProfileDir(startFlags.matched);
const rootProfileDir = determineProfileDir(startFlags.matched.profileDir, appName);
const { profileDir } = await initProfile(rootProfileDir);

// We can't access the database, so write to a file instead.
Expand Down
33 changes: 12 additions & 21 deletions packages/lib/BaseApplication.ts
Expand Up @@ -25,7 +25,6 @@ import { reg } from './registry';
import time from './time';
import BaseSyncTarget from './BaseSyncTarget';
import reduxSharedMiddleware from './components/shared/reduxSharedMiddleware';
const os = require('os');
import dns = require('dns');
import fs = require('fs-extra');
const EventEmitter = require('events');
Expand All @@ -48,7 +47,6 @@ import MigrationService from './services/MigrationService';
import ShareService from './services/share/ShareService';
import handleSyncStartupOperation from './services/synchronizer/utils/handleSyncStartupOperation';
import SyncTargetJoplinCloud from './SyncTargetJoplinCloud';
const { toSystemSlashes } = require('./path-utils');
const { setAutoFreeze } = require('immer');
import { getEncryptionEnabled } from './services/synchronizer/syncInfoUtils';
import { loadMasterKeysFromSettings, migrateMasterPassword } from './services/e2ee/utils';
Expand All @@ -62,16 +60,20 @@ import { parseShareCache } from './services/share/reducer';
import RotatingLogs from './RotatingLogs';
import { NoteEntity } from './services/database/types';
import { join } from 'path';
import processStartFlags, { MatchedStartFlags } from './utils/processStartFlags';
import processStartFlags from './utils/processStartFlags';
import determineProfileDir from './determineProfileDir';

const appLogger: LoggerWrapper = Logger.create('App');

// const ntpClient = require('./vendor/ntp-client');
// ntpClient.dgram = require('dgram');

interface StartOptions {
export interface StartOptions {
keychainEnabled?: boolean;
setupGlobalLogger?: boolean;
rootProfileDir?: string;
appName?: string;
appId?: string;
}
export const safeModeFlagFilename = 'force-safe-mode-on-next-start';

Expand Down Expand Up @@ -600,20 +602,6 @@ export default class BaseApplication {
return flags.matched;
}

public static determineProfileDir(initArgs: MatchedStartFlags) {
let output = '';

if (initArgs.profileDir) {
output = initArgs.profileDir;
} else if (process && process.env && process.env.PORTABLE_EXECUTABLE_DIR) {
output = `${process.env.PORTABLE_EXECUTABLE_DIR}/JoplinProfile`;
} else {
output = `${os.homedir()}/.config/${Setting.value('appName')}`;
}

return toSystemSlashes(output, 'linux');
}

protected startRotatingLogMaintenance(profileDir: string) {
this.rotatingLogs = new RotatingLogs(profileDir);
const processLogs = async () => {
Expand Down Expand Up @@ -641,14 +629,17 @@ export default class BaseApplication {
let initArgs = startFlags.matched;
if (argv.length) this.showPromptString_ = false;

let appName = initArgs.env === 'dev' ? 'joplindev' : 'joplin';
if (Setting.value('appId').indexOf('-desktop') >= 0) appName += '-desktop';
let appName = options.appName;
if (!appName) {
appName = initArgs.env === 'dev' ? 'joplindev' : 'joplin';
if (Setting.value('appId').indexOf('-desktop') >= 0) appName += '-desktop';
}
Setting.setConstant('appName', appName);

// https://immerjs.github.io/immer/docs/freezing
setAutoFreeze(initArgs.env === 'dev');

const rootProfileDir = BaseApplication.determineProfileDir(initArgs);
const rootProfileDir = options.rootProfileDir ? options.rootProfileDir : determineProfileDir(initArgs.profileDir, appName);
const { profileDir, profileConfig, isSubProfile } = await initProfile(rootProfileDir);
this.profileConfig_ = profileConfig;

Expand Down
16 changes: 16 additions & 0 deletions packages/lib/determineProfileDir.ts
@@ -0,0 +1,16 @@
import { homedir } from 'os';
import { toSystemSlashes } from './path-utils';

export default (profileFromArgs: string, appName: string) => {
let output = '';

if (profileFromArgs) {
output = profileFromArgs;
} else if (process && process.env && process.env.PORTABLE_EXECUTABLE_DIR) {
output = `${process.env.PORTABLE_EXECUTABLE_DIR}/JoplinProfile`;
} else {
output = `${homedir()}/.config/${appName}`;
}

return toSystemSlashes(output, 'linux');
};
4 changes: 3 additions & 1 deletion packages/lib/models/Setting.ts
Expand Up @@ -1391,7 +1391,9 @@ class Setting extends BaseModel {
public: true,
appTypes: [AppType.Desktop],
label: () => 'Automatically upload crash reports',
description: () => 'If you experience a crash, please enable this option to automatically send a crash report.',
description: () => 'If you experience a crash, please enable this option to automatically send crash reports. You will need to restart the application for this change to take effect.',
isGlobal: true,
storage: SettingStorage.File,
},

'clipperServer.autoStart': { value: false, type: SettingItemType.Bool, storage: SettingStorage.File, isGlobal: true, public: false },
Expand Down

0 comments on commit 07aba91

Please sign in to comment.