Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,25 @@ declare module 'papi-shared-types' {
* this command (e.g., Paratext 10 Studio)
*/
'paratextBibleSendReceive.syncProjects': (projectIds?: string[]) => Promise<void>;
/**
* Runs the Send/Receive sync scheduled for a session boundary ("On startup/shutdown") for the
* projects the user scheduled for that boundary. The extension owns the schedule, running the
* sync, and — on startup only — raising/clearing the block on editing a project while it is
* syncing; core only triggers it at the corresponding session boundary.
*
* Handles its own errors internally and always resolves (never rejects for an internal
* failure), reporting the outcome via the return value so the triggering side can log it
* truthfully.
*
* @param boundary Which session boundary is being triggered.
* @returns `'synced'` if a scheduled sync ran and completed, `'failed'` if one ran but did not
* complete successfully, or `'skipped'` if nothing ran (nothing scheduled, not due, or
* another sync already in progress).
* @experimental This command is unstable and may change or disappear without notice
*/
'paratextBibleSendReceive.runScheduledSessionSync': (
boundary: 'startup' | 'shutdown',
) => Promise<'synced' | 'failed' | 'skipped'>;
/**
* Accepts a project id, and returns a RevisionInfo[] of all revisions for the given project.
*
Expand Down
40 changes: 40 additions & 0 deletions lib/papi-dts/papi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,22 @@ declare module 'shared/data/rpc.model' {
} from 'json-rpc-2.0';
/** Port to use for the WebSocket */
export const WEBSOCKET_PORT = 8876;
/**
* How many times to try sending a request before giving up if the request is not yet registered.
* Exported so callers that layer their own retry policy on top of {@link requestWithRetry}'s cadence
* (e.g. the Power-mode startup sync's boot-race loop) can derive from this shared policy instead of
* re-declaring the literal and silently diverging if it is ever retuned.
*
* @experimental
*/
export const MAX_REQUEST_ATTEMPTS = 10;
/**
* How long in ms to wait between request attempts if the request is not yet registered. Exported
* for the same derive-don't-duplicate reason as {@link MAX_REQUEST_ATTEMPTS}.
*
* @experimental
*/
export const REQUEST_ATTEMPT_WAIT_TIME_MS = 1000;
/**
* Whether an RPC object is setting up or has finished setting up its connection and is ready to
* communicate on the network
Expand Down Expand Up @@ -1152,6 +1168,30 @@ declare module 'shared/data/rpc.model' {
export const GET_METHODS = 'rpc.discover';
/** Prefix on requests that indicates that the request is a command */
export const CATEGORY_COMMAND = 'command';
/**
* Builds the exact prefix that `network.service`'s `doRequest` embeds in the message it throws for
* a JSON-RPC _error response_ with the given `code` — the full thrown message is this prefix
* followed by `: <error message>`.
*
* Exported so the few callers that must classify these thrown errors by message (there is no richer
* machine-readable marker for a "method not found" response) derive the format from this single
* producer instead of hand-copying the literal. Hand-copied copies silently drift: reformat the
* producer and a separate matcher/fixture keeps matching its old string while real errors stop
* matching, and the tests stay green. Everything routing through this function stays in lockstep.
*
* @param code The JSON-RPC error code from the error response being classified
* @returns The exact message prefix `doRequest` uses for an error response with that `code`
* @experimental
*/
export function getJsonRpcRequestErrorMessagePrefix(code: number): string;
/**
* Prefix that `network.service`'s `doRequest` embeds in the message it throws when a request times
* out client-side before any response arrives. Exported for the same drift-prevention reason as
* {@link getJsonRpcRequestErrorMessagePrefix}.
*
* @experimental
*/
export const JSON_RPC_REQUEST_TIMED_OUT_MESSAGE_PREFIX = 'JSON-RPC Request timed out:';
}
declare module 'shared/models/openrpc.model' {
import type { JSONSchema7 } from 'json-schema';
Expand Down
42 changes: 36 additions & 6 deletions src/main/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,29 @@ async function main() {
// TODO (maybe): Wait for signal from the extension host process that it is ready (except 'getWebView')
// We could then wait for the renderer to be ready and signal the extension host

// Fire-and-forget startup tasks (e.g. simple-mode initial S/R). Must not block window creation.
// The S/R command is registered by the .NET data provider, which may not yet be reachable;
// performStartupTasks uses retrying sendCommand semantics and swallows failures internally.
// Signals for the fire-and-forget startup tasks: an abort controller so the Power-mode boot-race
// retry loop stops the moment the app begins quitting (wired below), and a window-interactive
// clock so a startup sync that only registers late isn't fired onto an editor the user is already
// using (see performStartupTasks / STARTUP_SYNC_FRESHNESS_WINDOW_MS).
const startupTasksAbort = new AbortController();
let mainWindowInteractiveAt: number | undefined;

// Fire-and-forget startup tasks (initial S/R). Must not block window creation. In Simple mode the
// S/R command is served by the .NET data provider and is driven through the retrying
// `commandService.sendCommand`. In Power mode the trigger is `runScheduledSessionSync`, which the
// send-receive extension registers in the extension host (not the .NET data provider) and which
// performStartupTasks drives via `requestNoRetry` inside its own bounded 120 s boot-race loop —
// deliberately NOT sendCommand's retry semantics. Either way failures are swallowed internally.
// Wrapped in an async IIFE per code-style preference for try/catch over `.catch()` chains.
(async () => {
try {
await performStartupTasks();
await performStartupTasks({
abortSignal: startupTasksAbort.signal,
getWindowInteractiveElapsedMs: () =>
mainWindowInteractiveAt === undefined
? undefined
: performance.now() - mainWindowInteractiveAt,
});
} catch (e) {
logger.warn(`performStartupTasks threw unexpectedly: ${getErrorMessage(e)}`);
}
Expand Down Expand Up @@ -476,6 +492,10 @@ async function main() {

mainWindow.on('ready-to-show', async () => {
logger.info('mainWindow is ready to show');
// Anchor the startup-sync freshness clock to when the window first becomes interactive (see
// the startup-tasks signals above): a late-registering startup sync is only fired if the user
// hasn't yet had the window long enough to be editing.
mainWindowInteractiveAt ??= performance.now();
if (!mainWindow) throw new Error('"mainWindow" is not defined');
if (process.env.START_MINIMIZED) {
logger.info('mainWindow is starting minimized due to START_MINIMIZED env variable');
Expand Down Expand Up @@ -555,13 +575,20 @@ async function main() {
// the window until the sync completes.
let isWindowClosing = false;
mainWindow.on('close', async (event) => {
// Prevents a "double close" when the user tries to press the close window button a second
// time
// A second close click while the first shutdown is still running falls through to Electron's
// default close on purpose: with the shutdown sync's request timeout disabled by the
// extension, the bounded wait below can hold the window up to SHUTDOWN_SYNC_TIME_OUT_MS with
// no feedback, and this fall-through is the user's only escape hatch until a real
// feedback/cancel UX exists (tracked on the shutdown-cancel follow-up ticket). It abandons
// the in-flight sync mid-flight — same risk profile as force-quitting the app.
if (isWindowClosing) return;

// Prevents the main window from initially closing
event.preventDefault();
isWindowClosing = true;
// The app is on its way down: stop the startup boot-race retry loop so it can't fire a startup
// sync after this shutdown sync, or reach a network connection that is about to be torn down.
startupTasksAbort.abort();

try {
await performShutdownTasks();
Expand Down Expand Up @@ -802,6 +829,9 @@ async function main() {
app.on('will-quit', async (e) => {
if (!isAppQuitting) {
logger.info('Main process is quitting');
// Stop the startup boot-race retry loop before networkService.shutdown() tears down the
// connection, so a late retry can't resurrect it (a no-op if the window close already aborted).
startupTasksAbort.abort();

// Prevent closing before graceful shutdown is complete.
// Also, in the future, this should allow a "are you sure?" dialog to display.
Expand Down
44 changes: 44 additions & 0 deletions src/main/scheduled-session-sync.util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { CATEGORY_COMMAND } from '@shared/data/rpc.model';
import { serializeRequestType } from '@shared/utils/util';

/**
* Fully-qualified name of the S/R extension command that runs the sync scheduled for a session
* boundary ("On startup/shutdown"). Shared by the startup and shutdown task modules so the two call
* sites can't drift apart: a typo in one bare string literal compiles cleanly and both suites still
* pass (they only assert `stringContaining('runScheduledSessionSync')`), so a single source for the
* name is the only thing that actually pins them together.
*
* This is deliberately NOT wired through the typed `commandService.sendCommand`. That path always
* applies the shared network retry policy, which both call sites exist specifically to avoid — the
* shutdown call wants no retry at all, and the startup call has its own longer boot-race budget.
* Typing the name here (via the extension's vendored `CommandHandlers` declaration) would buy the
* name check at the cost of the retry semantics `networkService.requestNoRetry` is chosen for, so
* this shared constant gives the name safety without giving up the raw request path.
*/
export const RUN_SCHEDULED_SESSION_SYNC_COMMAND =
'paratextBibleSendReceive.runScheduledSessionSync';

/** Serialized request type for {@link RUN_SCHEDULED_SESSION_SYNC_COMMAND}. */
export const RUN_SCHEDULED_SESSION_SYNC_REQUEST_TYPE = serializeRequestType(
CATEGORY_COMMAND,
RUN_SCHEDULED_SESSION_SYNC_COMMAND,
);

/** The two session boundaries at which a scheduled sync can be triggered. */
export type SessionSyncBoundary = 'startup' | 'shutdown';

/**
* Result the S/R extension's `runScheduledSessionSync` command resolves with. The command handles
* its own errors internally and always resolves (it never rejects for an internal failure), so core
* distinguishes the three real outcomes from this value rather than from resolve-vs-reject:
*
* - `synced`: a scheduled sync ran and completed successfully.
* - `failed`: a scheduled sync ran but did not complete successfully (the extension logged detail).
* - `skipped`: nothing ran — nothing was scheduled for this boundary, it wasn't due, or another sync
* was already in progress (the common default state until the user schedules something).
*
* A rejection from the call still means the command couldn't be reached/executed at all (not
* registered yet, or a request timeout); callers treat that separately from these settled
* outcomes.
*/
export type ScheduledSessionSyncResult = 'synced' | 'failed' | 'skipped';
116 changes: 103 additions & 13 deletions src/main/shutdown-tasks.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { vi } from 'vitest';
import { SHUTDOWN_SYNC_TIME_OUT_MS } from '@shared/data/platform.data';
import { settingsService } from '@shared/services/settings.service';
import * as networkService from '@shared/services/network.service';
import { networkObjectService } from '@shared/services/network-object.service';
import { logger } from '@shared/services/logger.service';
import { performShutdownTasks } from './shutdown-tasks';

vi.mock('@shared/services/settings.service', () => ({
Expand All @@ -17,12 +19,18 @@ vi.mock('@shared/services/network-object.service', () => ({
}));

vi.mock('@shared/services/logger.service', () => ({
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
// `debug` matches the sibling startup-tasks.test.ts mock; logShutdownSyncOutcome logs a scheduled
// skip at debug, so leaving it out would throw "logger.debug is not a function".
logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
}));

const mockSettingsGet = vi.mocked(settingsService.get);
const mockRequestNoRetry = vi.mocked(networkService.requestNoRetry);
const mockNetworkObjectGet = vi.mocked(networkObjectService.get);
Comment thread
lyonsil marked this conversation as resolved.
const mockLoggerDebug = vi.mocked(logger.debug);
const mockLoggerInfo = vi.mocked(logger.info);
const mockLoggerWarn = vi.mocked(logger.warn);
const mockLoggerError = vi.mocked(logger.error);

function makeWebViewService(defs: object[]) {
return {
Expand All @@ -37,10 +45,74 @@ beforeEach(() => {
});

describe('performShutdownTasks', () => {
it('returns without any network calls when interface mode is not simple', async () => {
it('fires runScheduledSessionSync("shutdown") and logs "complete" when the command reports "synced"', async () => {
mockSettingsGet.mockResolvedValue('power');
mockRequestNoRetry.mockResolvedValue('synced');
await performShutdownTasks();
expect(mockRequestNoRetry).not.toHaveBeenCalled();
expect(mockRequestNoRetry).toHaveBeenCalledWith(
expect.stringContaining('runScheduledSessionSync'),
'shutdown',
);
// Power mode selects by schedule, not open editors/cancelSync — neither Simple-mode call fires.
expect(mockRequestNoRetry.mock.calls.map(([cmd]) => cmd)).not.toContainEqual(
expect.stringContaining('cancelSync'),
);
expect(mockNetworkObjectGet).not.toHaveBeenCalled();
// A reported "synced" is the only thing that produces the truthful "complete" log.
expect(mockLoggerInfo).toHaveBeenCalledWith('Sync on shutdown complete');
});

it('treats a legacy void resolution as "synced" (logs "complete")', async () => {
mockSettingsGet.mockResolvedValue('power');
mockRequestNoRetry.mockResolvedValue(undefined);
await performShutdownTasks();
expect(mockLoggerInfo).toHaveBeenCalledWith('Sync on shutdown complete');
});

it('warns (does not log "complete") when the command reports "failed"', async () => {
mockSettingsGet.mockResolvedValue('power');
mockRequestNoRetry.mockResolvedValue('failed');
await performShutdownTasks();
expect(mockLoggerWarn).toHaveBeenCalledWith(expect.stringContaining('reported failure'));
expect(mockLoggerInfo).not.toHaveBeenCalledWith('Sync on shutdown complete');
});

it('logs a debug-only skip (no info, no warn) when the command reports "skipped"', async () => {
mockSettingsGet.mockResolvedValue('power');
mockRequestNoRetry.mockResolvedValue('skipped');
await performShutdownTasks();
expect(mockLoggerDebug).toHaveBeenCalledWith(expect.stringContaining('skipped'));
expect(mockLoggerInfo).not.toHaveBeenCalledWith('Sync on shutdown complete');
expect(mockLoggerWarn).not.toHaveBeenCalled();
});

it('swallows a missing/failing runScheduledSessionSync command in power mode without throwing, and does not log "complete"', async () => {
mockSettingsGet.mockResolvedValue('power');
mockRequestNoRetry.mockRejectedValue(new Error('command not registered'));
await expect(performShutdownTasks()).resolves.toBeUndefined();
// The bounded wait settled (the rejection was caught, not a timeout), so the failure is warned
// and the misleading "complete" is never logged.
expect(mockLoggerWarn).toHaveBeenCalledWith(expect.stringContaining('failed or skipped'));
expect(mockLoggerInfo).not.toHaveBeenCalledWith('Sync on shutdown complete');
});

it('returns after the timeout when runScheduledSessionSync never settles, logs the timeout, and never logs "complete"', async () => {
mockSettingsGet.mockResolvedValue('power');
// Never-resolving promise: simulates a genuinely hung sync. `performSync` never settles, so the
// AsyncVariable's timeout rejects `syncComplete.promise` and runBoundedShutdownSync takes its
// `timedOut` branch — the one path SHUTDOWN_SYNC_TIME_OUT_MS exists to bound. (An unregistered
// command does NOT reach here; it rejects fast and settles as `failed`, covered above.)
mockRequestNoRetry.mockImplementation(() => new Promise(() => {}));
vi.useFakeTimers();
try {
const shutdownPromise = performShutdownTasks();
await vi.advanceTimersByTimeAsync(SHUTDOWN_SYNC_TIME_OUT_MS);
await expect(shutdownPromise).resolves.toBeUndefined();
} finally {
vi.useRealTimers();
}
expect(mockLoggerWarn).toHaveBeenCalledWith(expect.stringContaining('timed out'));
expect(mockLoggerInfo).not.toHaveBeenCalledWith(expect.stringContaining('complete'));
});

it('cancels sync but skips S/R when no Scripture Editor is open', async () => {
Expand Down Expand Up @@ -97,7 +169,9 @@ describe('performShutdownTasks', () => {
);
});

it('proceeds with S/R (simple-mode fallback) when settings service throws', async () => {
it('skips the automatic shutdown S/R and warns when settings service throws (no open-editor fallback)', async () => {
// Symmetric with startup: an unreadable mode must not fall through to Simple's open-editor S/R,
// which could sync a project a Power user excluded from their schedule. Do nothing and warn.
mockSettingsGet.mockRejectedValue(new Error('settings unavailable'));
mockNetworkObjectGet.mockResolvedValue(
makeWebViewService([
Expand All @@ -109,20 +183,36 @@ describe('performShutdownTasks', () => {
]),
);
await performShutdownTasks();
expect(mockRequestNoRetry).toHaveBeenCalledWith(expect.stringContaining('cancelSync'));
expect(mockRequestNoRetry).toHaveBeenCalledWith(
expect.stringContaining('sendReceiveProjects'),
['fallback-project'],
expect(mockRequestNoRetry).not.toHaveBeenCalled();
expect(mockNetworkObjectGet).not.toHaveBeenCalled();
expect(mockLoggerWarn).toHaveBeenCalledWith(
expect.stringContaining('Could not read platform.interfaceMode'),
);
});

it('swallows unexpected errors and does not throw', async () => {
it('swallows unexpected errors and does not throw (exercises the outer try/catch)', async () => {
mockSettingsGet.mockResolvedValue('simple');
mockNetworkObjectGet.mockResolvedValue(makeWebViewService([]));
// Force an unexpected throw deep inside by making cancelSync throw a non-Error value
mockRequestNoRetry.mockImplementation(() => {
throw new Error('unexpected non-error throw');
// A WRITABLE editor so the flow runs past the `if (!projectId) return` early return and into
// performSimpleModeShutdownSync's unguarded region (an empty list would return early and never
// exercise the outer catch — the whole point of this test).
mockNetworkObjectGet.mockResolvedValue(
makeWebViewService([
{
webViewType: 'platformScriptureEditor.react',
state: { isReadOnly: false },
projectId: 'p1',
},
]),
);
// Inject an unexpected error from an UNGUARDED spot — the "Syncing project on shutdown..." log,
// which is not wrapped in an inner try/catch. It escapes the inner handlers so that only
// performShutdownTasks's outer try/catch can swallow it. (Verified falsifiable: deleting that
// outer try/catch makes performShutdownTasks reject and this `resolves` assertion fail.)
mockLoggerInfo.mockImplementationOnce(() => {
throw new Error('unexpected logging failure');
});
await expect(performShutdownTasks()).resolves.toBeUndefined();
// The outer catch handled it via logger.error.
expect(mockLoggerError).toHaveBeenCalled();
});
});
Loading
Loading