Skip to content

Commit a054f8f

Browse files
johannrichardjohannrichard
committed
refactor: 🐛 improve robustness of notification handling
1 parent bff065e commit a054f8f

8 files changed

Lines changed: 72 additions & 67 deletions

File tree

‎src/main.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { DeduplicatingVaultWriter } from 'services/deduplicating-vault-writer';
99
import { Frontmatter } from 'services/frontmatter';
1010
import { FrontmatterManager } from 'services/frontmatter-manager';
1111
import Logger from 'services/logger';
12-
import type ReadwiseApi from 'services/readwise-api';
1312
import { ReadwiseEnvironment, ReadwiseLoader } from 'services/readwise-environment';
1413
import spacetime from 'spacetime';
1514
import type { BaseFile, ReadwiseDocument } from 'types/document';
@@ -23,12 +22,11 @@ import { createdDate, lastHighlightedDate, updatedDate } from 'utils/highlight-d
2322
import type { PluginSettings } from './types/settings';
2423

2524
export default class ReadwiseMirror extends Plugin {
25+
private notify: Notify;
2626
private settings: PluginSettings;
27-
private readwiseApi: ReadwiseApi;
2827
private loader: ReadwiseLoader;
2928
private env: ReadwiseEnvironment;
3029
private logger: Logger;
31-
private notify: Notify;
3230
private lock: Lock<string>;
3331
private frontmatterManager: FrontmatterManager;
3432
private deduplicatingVaultWriter: DeduplicatingVaultWriter;
@@ -49,9 +47,12 @@ export default class ReadwiseMirror extends Plugin {
4947
settings: this.settings,
5048
app: this.app,
5149
logger: this.logger,
52-
notify: this.notify,
53-
saveAndApplySettings: this.saveAndApplySettings.bind(this),
5450
syncLock: this.lock,
51+
statusBarItem: this.notify.statusBarItem,
52+
// exposed methods
53+
notice: this.notify.notice.bind(this.notify),
54+
setStatusBarText: this.notify.setStatusBarText.bind(this.notify),
55+
saveAndApplySettings: this.saveAndApplySettings.bind(this),
5556
};
5657
return ctx;
5758
}

‎src/services/command-manager.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export class CommandManager {
5454
this.ctx.app.workspace.on('file-menu', (menu, file) => this.onMenuOpenCallback(menu, file))
5555
);
5656

57-
this.plugin.registerDomEvent(this.ctx.notify.statusBarItem, 'click', async () => await this.ctr.sync());
57+
this.plugin.registerDomEvent(this.ctx.statusBarItem, 'click', async () => await this.ctr.sync());
5858
}
5959

6060
public runStartupCommands(): void {
@@ -95,7 +95,7 @@ export class CommandManager {
9595
.setTitle('Copy Readwise URL')
9696
.onClick(async () => {
9797
await navigator.clipboard.writeText(trackingUrl);
98-
this.ctx.notify.notice('Readwise: URL copied to clipboard');
98+
this.ctx.notice('Readwise: URL copied to clipboard');
9999
});
100100
});
101101
}

‎src/services/controller.ts‎

Lines changed: 45 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export class Controller {
3232
} catch (err) {
3333
// Keep instance but log/notify — callers must still check api presence/validity.
3434
Controller.instance.ctx.logger.error('ReadwiseController: failed to create API instance', err);
35-
Controller.instance.ctx.notify.notice('Readwise: Failed to initialize API. Check settings.');
35+
Controller.instance.ctx.notice('Readwise: Failed to initialize API. Check settings.');
3636
Controller.instance.api = undefined;
3737
}
3838
return Controller.instance;
@@ -61,37 +61,42 @@ export class Controller {
6161
public async sync() {
6262
// Equivalent to plugin.sync()
6363
if (this.ctx.syncLock?.isAcquired('library-sync')) {
64-
this.ctx.notify.notice('Sync already in progress');
64+
this.ctx.notice('Sync already in progress');
6565
return;
6666
}
6767
await this.ctx.syncLock?.acquire('library-sync');
6868
try {
69-
if (!(await Controller.validateAPIInstance())) {
70-
this.ctx.notify.notice('Readwise: Network connection and valid API Token required');
71-
return;
72-
}
7369
let library: Library;
7470
if (!this.ctx.settings.lastUpdated) {
7571
if (this.ctx.settings.syncNotifications)
76-
this.ctx.notify.notice('Readwise: Previous sync not detected...\nDownloading full Readwise library');
72+
this.ctx.notice('Readwise: Previous sync not detected...\nDownloading full Readwise library');
73+
if (!(await Controller.validateAPIInstance())) {
74+
this.ctx.notice('Readwise: Network connection and valid API Token required');
75+
return;
76+
}
7777
library = await this.api.downloadFullLibrary();
7878
} else {
79-
if (this.ctx.settings.syncNotifications)
80-
this.ctx.notify.notice(
79+
if (this.ctx.settings.syncNotifications) {
80+
this.ctx.notice(
8181
`Readwise: Checking for new updates since ${humanReadableFormat(this.ctx.settings.lastUpdated)}...`
8282
);
83+
}
84+
if (!(await Controller.validateAPIInstance())) {
85+
this.ctx.notice('Readwise: Network connection and valid API Token required');
86+
return;
87+
}
8388
library = await this.api.downloadUpdates(this.ctx.settings.lastUpdated);
8489
}
8590
// ...existing filtering and writing logic...
8691
await this.plugin.writeLibraryToMarkdown(library);
8792
if (this.ctx.settings.logFile) await this.plugin.writeLogToMarkdown(library);
8893
this.ctx.settings.lastUpdated = new Date().toISOString();
8994
await this.ctx.saveAndApplySettings();
90-
this.ctx.notify.setStatusBarText(`Readwise: Synced ${humanReadableFormat(this.ctx.settings.lastUpdated)}`);
95+
this.ctx.setStatusBarText(`Readwise: Synced ${humanReadableFormat(this.ctx.settings.lastUpdated)}`);
9196
} catch (error) {
9297
this.ctx.logger.error('Error during sync:', error);
93-
this.ctx.notify.notice(`Readwise: Sync failed. ${error}`);
94-
this.ctx.notify.setStatusBarText(`Readwise: Sync error ${error}`);
98+
this.ctx.notice(`Readwise: Sync failed. ${error}`);
99+
this.ctx.setStatusBarText(`Readwise: Sync error ${error}`);
95100
} finally {
96101
this.ctx.syncLock?.release('library-sync');
97102
}
@@ -108,31 +113,31 @@ export class Controller {
108113
try {
109114
this.ctx.logger.debug('Attempting to delete entire library at:', abstractFile);
110115
await this.ctx.app.fileManager.trashFile(abstractFile);
111-
if (this.ctx.settings.syncNotifications) this.ctx.notify.notice('Readwise: library folder deleted');
116+
if (this.ctx.settings.syncNotifications) this.ctx.notice('Readwise: library folder deleted');
112117
} catch (err) {
113118
this.ctx.logger.error(`Attempted to delete file ${path} but no file was found`, err);
114-
if (this.ctx.settings.syncNotifications) this.ctx.notify.notice('Readwise: Error deleting library folder');
119+
if (this.ctx.settings.syncNotifications) this.ctx.notice('Readwise: Error deleting library folder');
115120
}
116121
}
117-
this.ctx.notify.setStatusBarText('Readwise: Click to Sync');
122+
this.ctx.setStatusBarText('Readwise: Click to Sync');
118123
}
119124

120125
/**
121126
* Update current note with Readwise data
122127
*/
123128
public async updateSingleNote(trackedFile: TTrackedFile): Promise<void> {
124129
if (this.ctx.syncLock.isAcquired(trackedFile.readwiseId.toString())) {
125-
this.ctx.notify.notice('Readwise: Update already in progress');
130+
this.ctx.notice('Readwise: Update already in progress');
126131
return;
127132
}
128133

129134
if (!(await Controller.validateAPIInstance())) {
130-
this.ctx.notify.notice('Readwise: Network connection and valid API Token required');
135+
this.ctx.notice('Readwise: Network connection and valid API Token required');
131136
return;
132137
}
133138

134139
if (!trackedFile.isUpdatable) {
135-
this.ctx.notify.notice('Readwise: Current note is not a tracked Readwise note.');
140+
this.ctx.notice('Readwise: Current note is not a tracked Readwise note.');
136141
return;
137142
}
138143

@@ -152,15 +157,15 @@ export class Controller {
152157

153158
if (this.ctx.settings.logFile) await this.plugin.writeLogToMarkdown(library);
154159

155-
if (this.ctx.settings.syncNotifications) this.ctx.notify.notice('Readwise: Book update complete.');
160+
if (this.ctx.settings.syncNotifications) this.ctx.notice('Readwise: Book update complete.');
156161
} else {
157-
this.ctx.notify.notice(`Readwise: Note with id ${trackedFile.readwiseId} not found on Readwise.`);
162+
this.ctx.notice(`Readwise: Note with id ${trackedFile.readwiseId} not found on Readwise.`);
158163
this.ctx.logger.warn(`Readwise: Note with id ${trackedFile.readwiseId} not found on Readwise.`);
159164
return;
160165
}
161166
} catch (error) {
162167
this.ctx.logger.error('Error during single-book update:', error);
163-
this.ctx.notify.notice(`Readwise: Sync failed. ${error}`);
168+
this.ctx.notice(`Readwise: Sync failed. ${error}`);
164169
} finally {
165170
// Make sure we release the lock even if the operation fails
166171
this.ctx.syncLock.release(trackedFile.readwiseId.toString());
@@ -170,14 +175,14 @@ export class Controller {
170175
public async updateAllFrontmatter() {
171176
// Equivalent to plugin.updateAllFrontmatter()
172177
if (this.ctx.syncLock?.isAcquired('frontmatter-update')) {
173-
this.ctx.notify.notice('Readwise: update already in progress');
178+
this.ctx.notice('Readwise: update already in progress');
174179
return;
175180
}
176181
if (!(await Controller.validateAPIInstance())) {
177-
this.ctx.notify.notice('Readwise: Network connection and valid API Token required');
182+
this.ctx.notice('Readwise: Network connection and valid API Token required');
178183
return;
179184
}
180-
this.ctx.notify.notice('Readwise: Updating all note frontmatter...');
185+
this.ctx.notice('Readwise: Updating all note frontmatter...');
181186
await this.ctx.syncLock?.acquire('frontmatter-update');
182187
try {
183188
this.ctx.logger.debug('Readwise: downloading full library to update frontmatter...');
@@ -188,10 +193,10 @@ export class Controller {
188193
if (this.ctx.settings.filterNotesByTag && this.ctx.settings.filteredTags?.length > 0) {
189194
message += ` (filtered by tags: ${this.ctx.settings.filteredTags.join(', ')})`;
190195
}
191-
this.ctx.notify.notice(message);
196+
this.ctx.notice(message);
192197
} catch (error) {
193198
this.ctx.logger.error('Error during frontmatter sync:', error);
194-
this.ctx.notify.notice(`Readwise: Sync failed. ${error}`);
199+
this.ctx.notice(`Readwise: Sync failed. ${error}`);
195200
} finally {
196201
this.ctx.syncLock?.release('frontmatter-update');
197202
}
@@ -205,13 +210,13 @@ export class Controller {
205210
const path = `${this.ctx.settings.baseFolderName}`;
206211
const readwiseFolder = vault.getAbstractFileByPath(path);
207212
if (readwiseFolder && readwiseFolder instanceof TFolder) {
208-
this.ctx.notify.notice('Readwise: Filename adjustment started');
213+
this.ctx.notice('Readwise: Filename adjustment started');
209214
// Iterate all files in the Readwise folder and "fix" their names according to the current settings using
210215
const renamedFiles = await this.iterativeReadwiseRenamer(readwiseFolder);
211216
if (renamedFiles > 0) {
212-
this.ctx.notify.notice(`Readwise: Renamed ${renamedFiles} files. Check console for renaming errors.`);
217+
this.ctx.notice(`Readwise: Renamed ${renamedFiles} files. Check console for renaming errors.`);
213218
} else {
214-
this.ctx.notify.notice('Readwise: No files renamed. Check console for renaming errors.');
219+
this.ctx.notice('Readwise: No files renamed. Check console for renaming errors.');
215220
}
216221
}
217222
}
@@ -281,12 +286,12 @@ export class Controller {
281286
*/
282287
public async syncFolder(folder: TFolder) {
283288
if (this.ctx.syncLock.isAcquired('folder-sync')) {
284-
this.ctx.notify.notice('Readwise: sync already in progress');
289+
this.ctx.notice('Readwise: sync already in progress');
285290
return;
286291
}
287292

288293
if (!(await Controller.validateAPIInstance())) {
289-
this.ctx.notify.notice('Readwise: Network connection and valid API Token required');
294+
this.ctx.notice('Readwise: Network connection and valid API Token required');
290295
return;
291296
}
292297

@@ -304,7 +309,7 @@ export class Controller {
304309
const bookIds = trackedNotes.map((tracked) => tracked.readwiseId);
305310

306311
try {
307-
this.ctx.notify.notice(
312+
this.ctx.notice(
308313
`Readwise: Updating ${bookIds.length} note${bookIds.length !== 1 ? 's' : ''} in "${folder.name}"...`
309314
);
310315

@@ -313,12 +318,10 @@ export class Controller {
313318
this.ctx.logger.warn('Failed to update multiple files', error);
314319
}
315320

316-
this.ctx.notify.notice(
317-
`Readwise: Updated ${bookIds.length} note${bookIds.length !== 1 ? 's' : ''} in "${folder.name}"`
318-
);
321+
this.ctx.notice(`Readwise: Updated ${bookIds.length} note${bookIds.length !== 1 ? 's' : ''} in "${folder.name}"`);
319322
} catch (error) {
320323
this.ctx.logger.error('Error syncing folder:', error);
321-
this.ctx.notify.notice(`Readwise: Sync failed. ${error}`);
324+
this.ctx.notice(`Readwise: Sync failed. ${error}`);
322325
} finally {
323326
this.ctx.syncLock.release('folder-sync');
324327
}
@@ -351,12 +354,12 @@ export class Controller {
351354
*/
352355
private async updateMultipleNotes(bookIds: number[]): Promise<void> {
353356
if (this.ctx.syncLock.isAcquired('multiple-note-update')) {
354-
this.ctx.notify.notice('Readwise: Update for this note already in progress');
357+
this.ctx.notice('Readwise: Update for this note already in progress');
355358
return;
356359
}
357360

358361
if (!(await Controller.validateAPIInstance())) {
359-
this.ctx.notify.notice('Readwise: Network connection and valid API Token required');
362+
this.ctx.notice('Readwise: Network connection and valid API Token required');
360363
return;
361364
}
362365

@@ -373,19 +376,19 @@ export class Controller {
373376
}
374377

375378
if (this.ctx.settings.syncNotifications)
376-
this.ctx.notify.notice(`Readwise: writing ${Object.keys(library.books).length} updated books to markdown...`);
379+
this.ctx.notice(`Readwise: writing ${Object.keys(library.books).length} updated books to markdown...`);
377380
this.ctx.logger.debug(`Readwise: writing ${Object.keys(library.books).length} updated books to markdown...`);
378381
await this.plugin.writeLibraryToMarkdown(library);
379382
if (this.ctx.settings.logFile) await this.plugin.writeLogToMarkdown(library);
380-
if (this.ctx.settings.syncNotifications) this.ctx.notify.notice('Readwise: Book update complete.');
383+
if (this.ctx.settings.syncNotifications) this.ctx.notice('Readwise: Book update complete.');
381384
} else {
382-
this.ctx.notify.notice('Readwise: No notes from folder found on Readwise.');
385+
this.ctx.notice('Readwise: No notes from folder found on Readwise.');
383386
this.ctx.logger.warn('Readwise: No notes from folder found on Readwise.');
384387
return;
385388
}
386389
} catch (error) {
387390
this.ctx.logger.error('Error during multiple-book update:', error);
388-
this.ctx.notify.notice(`Readwise: Sync failed. ${error}`);
391+
this.ctx.notice(`Readwise: Sync failed. ${error}`);
389392
} finally {
390393
// Make sure we release the lock even if the operation fails
391394
this.ctx.syncLock.release('multiple-note-update');

‎src/services/deduplicating-vault-writer.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export class DeduplicatingVaultWriter {
2121

2222
private notifyFileCount() {
2323
this.fileCount++;
24-
this.ctx.notify.setStatusBarText(`Readwise: ${this.fileCount} of ${this.totalFileCount} files processed`);
24+
this.ctx.setStatusBarText(`Readwise: ${this.fileCount} of ${this.totalFileCount} files processed`);
2525
}
2626

2727
/**
@@ -197,7 +197,7 @@ export class DeduplicatingVaultWriter {
197197
this.totalFileCount = readwiseFiles.length;
198198
this.fileCount = 0;
199199

200-
this.ctx.notify.setStatusBarText(`Readwise: ${this.totalFileCount} files to process`);
200+
this.ctx.setStatusBarText(`Readwise: ${this.totalFileCount} files to process`);
201201

202202
// Group by path (which includes category and filename)
203203
const groupedByPath = new Map<string, BaseFile[]>();
@@ -227,7 +227,7 @@ export class DeduplicatingVaultWriter {
227227
this.totalFileCount = readwiseFiles.length;
228228
this.fileCount = 0;
229229

230-
this.ctx.notify.setStatusBarText(`Readwise: ${this.totalFileCount} files to process`);
230+
this.ctx.setStatusBarText(`Readwise: ${this.totalFileCount} files to process`);
231231

232232
// Process each file
233233
for (const readwiseFile of readwiseFiles) {

‎src/services/readwise-api.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export default class ReadwiseApi {
118118
if (bookId) this.ctx.logger.debug(`Checking for all highlights on book ID: ${bookId}`);
119119
let statusBarText = `Readwise: Fetching ${contentType}`;
120120
if (data?.count) statusBarText += ` (${results.length})`;
121-
this.ctx.notify.setStatusBarText(statusBarText);
121+
this.ctx.setStatusBarText(statusBarText);
122122

123123
// FIXME: When fetching very long period of data, the request might fail due to an URL which is too long (Error 414)
124124
const response: RequestUrlResponse = await requestUrl({ url: url + queryParams.toString(), ...this.options });
@@ -139,11 +139,11 @@ export default class ReadwiseApi {
139139
} else {
140140
this.ctx.logger.warn(`API Rate Limited, waiting to retry for ${rateLimitedDelayTime}`);
141141
}
142-
this.ctx.notify.setStatusBarText(`Readwise: API Rate Limited, waiting ${rateLimitedDelayTime}`);
142+
this.ctx.setStatusBarText(`Readwise: API Rate Limited, waiting ${rateLimitedDelayTime}`);
143143

144144
await new Promise((_) => setTimeout(_, rateLimitedDelayTime));
145145
this.ctx.logger.debug('Trying to fetch highlights again...');
146-
this.ctx.notify.setStatusBarText('Readwise: Attempting to retry...');
146+
this.ctx.setStatusBarText('Readwise: Attempting to retry...');
147147
} else {
148148
if (data.results && Array.isArray(data.results)) {
149149
results.push(...data.results);

‎src/types/plugin-context.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { App } from 'obsidian';
22
import type Logger from 'services/logger';
33
import type { PluginSettings } from 'types/settings';
4-
import type Notify from 'ui/notify';
54
import type { Controller } from '../services/controller';
65

76
/**
@@ -12,12 +11,14 @@ export interface PluginContext {
1211
app: App;
1312
settings: PluginSettings;
1413
logger: Logger;
15-
notify?: Notify;
1614
controller?: Controller;
1715
syncLock: {
1816
isAcquired(key: string): boolean;
1917
acquire(key: string): Promise<void>;
2018
release(key: string): void;
2119
};
20+
statusBarItem: HTMLElement;
21+
notice: (message: string, duration?: number) => void;
22+
setStatusBarText: (message: string) => void;
2223
saveAndApplySettings: () => Promise<void>;
2324
}

0 commit comments

Comments
 (0)