diff --git a/docs/long-term-plans/sync-core-extraction-plan.md b/docs/long-term-plans/sync-core-extraction-plan.md index 2eeddc84e2..ca61f375a4 100644 --- a/docs/long-term-plans/sync-core-extraction-plan.md +++ b/docs/long-term-plans/sync-core-extraction-plan.md @@ -104,6 +104,7 @@ groundwork: `packages/sync-core/**/*.ts`. - The package currently exports operation primitives, apply types, LWW helper factory, full-state op-type helper factory, entity-key helpers, + host-configured sync-file prefix helpers, generic error-message helpers, `SyncStateCorruptedError`, entity-registry contracts, and the privacy-aware logger port. - The app registry now has `buildEntityRegistry()` and an `ENTITY_REGISTRY` @@ -147,8 +148,9 @@ Suggested next order: 2. Continue targeted `SyncLogger` routing for files as they become movable. 3. Treat the PR 3b pure conflict-resolution and sync-import slices as complete for this round. -4. Clean up logger/config dependencies before moving compression, prefix, or - error helpers. +4. Continue logger/config cleanup before moving compression helpers or app + error classes; prefix parsing/formatting and generic error-message + extraction now have package-side helpers with app-owned diagnostics. 5. Defer port/orchestration work until the remaining pure/config boundaries are settled. @@ -284,6 +286,14 @@ Already present: entity IDs are still an SP replay convention. - `packages/sync-core/src/sync-logger.ts` defines `SyncLogger`, `NOOP_SYNC_LOGGER`, `SyncLogMeta`, `SyncLogError`, and `toSyncLogError()`. +- `packages/sync-core/src/sync-file-prefix.ts` defines + `createSyncFilePrefixHelpers()`. The app shim supplies + `REMOTE_FILE_CONTENT_PREFIX` and `InvalidFilePrefixError`, keeping SP storage + constants and diagnostics app-side while moving the generic parsing/formatting + logic behind a config boundary. +- `packages/sync-core/src/error.util.ts` defines `extractErrorMessage()` for + generic thrown-value message extraction. The app error module re-exports it + for compatibility while keeping SP/provider-specific error classes app-side. - `src/app/op-log/core/sync-logger.adapter.ts` wires `SyncLogger` to `OpLog` via the app-side `SYNC_LOGGER` injection token and the `OP_LOG_SYNC_LOGGER` direct adapter. @@ -410,11 +420,13 @@ Initial candidate-file audit: `toSyncLogError()` and preserves only safe counts such as input length. It remains app-side until compression errors are split from app diagnostics. - `op-log/core/errors/sync-errors.ts`: not move-ready. Several constructors log - validation params, raw samples, or additional objects; split generic error - classes from app-specific diagnostics before routing through `SyncLogger`. -- `op-log/util/sync-file-prefix.ts`: generic logic, but still depends on - app-side provider constants and app error classes. Move only after prefix and - error construction become package inputs. + validation params, raw samples, or additional objects. Generic + `extractErrorMessage()` now lives in the package, but error classes with + app-specific diagnostics stay app-side until their logging can be split or + routed safely through `SyncLogger`. +- `op-log/util/sync-file-prefix.ts`: now delegates to the package helper with + app-supplied prefix and error construction. The app-facing shim should remain + until consumers are deliberately switched to injected/configured helpers. ### What This Unlocks diff --git a/packages/sync-core/src/error.util.ts b/packages/sync-core/src/error.util.ts new file mode 100644 index 0000000000..bd62aeb63d --- /dev/null +++ b/packages/sync-core/src/error.util.ts @@ -0,0 +1,41 @@ +/** + * Extracts a meaningful error message from common thrown-value shapes. + * + * This helper intentionally does not log. Hosts decide whether an extracted + * message is safe for their user-visible or exportable diagnostics. + */ +export const extractErrorMessage = (err: unknown): string | null => { + if (typeof err === 'string' && err.length > 0) { + return err; + } + + if (err instanceof Error) { + const cause = (err as Error & { cause?: unknown }).cause; + if (cause instanceof Error && cause.message) { + return cause.message; + } + + const code = (err as Error & { code?: string }).code; + if (typeof code === 'string' && code.length > 0) { + return code; + } + + if (err.message && err.message.length > 0) { + return err.message; + } + } + + if ( + err !== null && + typeof err === 'object' && + 'message' in err && + typeof (err as { message: unknown }).message === 'string' + ) { + const msg = (err as { message: string }).message; + if (msg.length > 0) { + return msg; + } + } + + return null; +}; diff --git a/packages/sync-core/src/index.ts b/packages/sync-core/src/index.ts index f0253842c3..0832288e1c 100644 --- a/packages/sync-core/src/index.ts +++ b/packages/sync-core/src/index.ts @@ -29,6 +29,23 @@ export type { SyncImportFilterKeepReason, } from './sync-import-filter'; +// Host-configured sync file prefix helpers. +export { + createSyncFilePrefixHelpers, + SyncFilePrefixError, + SyncFilePrefixVersionError, +} from './sync-file-prefix'; +export type { + SyncFilePrefixConfig, + SyncFilePrefixHelpers, + SyncFilePrefixInvalidPrefixDetails, + SyncFilePrefixParams, + SyncFilePrefixParamsOutput, +} from './sync-file-prefix'; + +// Generic error helpers. +export { extractErrorMessage } from './error.util'; + // Full-state operation classification helper. Hosts supply their own op strings. export { FULL_STATE_OP_TYPES, diff --git a/packages/sync-core/src/sync-file-prefix.ts b/packages/sync-core/src/sync-file-prefix.ts new file mode 100644 index 0000000000..9c0fabc6e8 --- /dev/null +++ b/packages/sync-core/src/sync-file-prefix.ts @@ -0,0 +1,115 @@ +export interface SyncFilePrefixParams { + isCompress: boolean; + isEncrypt: boolean; + modelVersion: number; +} + +export interface SyncFilePrefixParamsOutput { + isCompressed: boolean; + isEncrypted: boolean; + modelVersion: number; + cleanDataStr: string; +} + +export interface SyncFilePrefixHelpers { + getSyncFilePrefix(cfg: SyncFilePrefixParams): string; + extractSyncFileStateFromPrefix(dataStr: string): SyncFilePrefixParamsOutput; +} + +export interface SyncFilePrefixConfig { + prefix: string; + endSeparator?: string; + createInvalidPrefixError?: (details: SyncFilePrefixInvalidPrefixDetails) => Error; +} + +export interface SyncFilePrefixInvalidPrefixDetails { + expectedPrefix: string; + endSeparator: string; + inputLength: number; +} + +export class SyncFilePrefixError extends Error { + override name = 'SyncFilePrefixError'; + + constructor(details: SyncFilePrefixInvalidPrefixDetails) { + super(`Invalid sync file prefix. Expected prefix "${details.expectedPrefix}".`); + } +} + +export class SyncFilePrefixVersionError extends Error { + override name = 'SyncFilePrefixVersionError'; + + constructor(modelVersion: number | string) { + const formattedModelVersion = String(modelVersion); + const safeModelVersion = + formattedModelVersion.length > 40 + ? `${formattedModelVersion.slice(0, 40)}...` + : formattedModelVersion; + super(`Invalid sync file model version: ${safeModelVersion}`); + } +} + +const DEFAULT_END_SEPARATOR = '__'; +const MODEL_VERSION_PATTERN = /^\d+(?:\.\d+)?$/; + +const escapeRegExp = (value: string): string => + value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +const formatModelVersion = (modelVersion: number): string => { + const formatted = String(modelVersion); + if ( + !Number.isFinite(modelVersion) || + modelVersion < 0 || + !MODEL_VERSION_PATTERN.test(formatted) + ) { + throw new SyncFilePrefixVersionError(modelVersion); + } + return formatted; +}; + +const parseModelVersion = (rawModelVersion: string): number => { + const modelVersion = parseFloat(rawModelVersion); + if (!Number.isFinite(modelVersion)) { + throw new SyncFilePrefixVersionError(rawModelVersion); + } + return modelVersion; +}; + +export const createSyncFilePrefixHelpers = ({ + prefix, + endSeparator = DEFAULT_END_SEPARATOR, + createInvalidPrefixError, +}: SyncFilePrefixConfig): SyncFilePrefixHelpers => { + const prefixPattern = escapeRegExp(prefix); + const separatorPattern = escapeRegExp(endSeparator); + const prefixRegex = new RegExp( + `^${prefixPattern}(C)?(E)?(\\d+(?:\\.\\d+)?)${separatorPattern}`, + ); + + return { + getSyncFilePrefix: (cfg: SyncFilePrefixParams): string => { + const c = cfg.isCompress ? 'C' : ''; + const e = cfg.isEncrypt ? 'E' : ''; + return `${prefix}${c}${e}${formatModelVersion(cfg.modelVersion)}${endSeparator}`; + }, + + extractSyncFileStateFromPrefix: (dataStr: string): SyncFilePrefixParamsOutput => { + const match = dataStr.match(prefixRegex); + if (!match) { + const details: SyncFilePrefixInvalidPrefixDetails = { + expectedPrefix: prefix, + endSeparator, + inputLength: dataStr.length, + }; + throw createInvalidPrefixError?.(details) ?? new SyncFilePrefixError(details); + } + + return { + isCompressed: !!match[1], + isEncrypted: !!match[2], + modelVersion: parseModelVersion(match[3]), + cleanDataStr: dataStr.slice(match[0].length), + }; + }, + }; +}; diff --git a/packages/sync-core/tests/error.util.spec.ts b/packages/sync-core/tests/error.util.spec.ts new file mode 100644 index 0000000000..ecb5f92876 --- /dev/null +++ b/packages/sync-core/tests/error.util.spec.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest'; +import { extractErrorMessage } from '../src'; + +describe('extractErrorMessage', () => { + it('returns non-empty string errors', () => { + expect(extractErrorMessage('plain failure')).toBe('plain failure'); + expect(extractErrorMessage('')).toBeNull(); + }); + + it('prefers an Error cause message when present', () => { + const err = new Error('outer message', { + cause: new Error('inner cause'), + }); + + expect(extractErrorMessage(err)).toBe('inner cause'); + }); + + it('returns error codes without host-specific normalization', () => { + const err = new Error(''); + Object.defineProperty(err, 'code', { + value: 'Z_BUF_ERROR', + }); + + expect(extractErrorMessage(err)).toBe('Z_BUF_ERROR'); + }); + + it('returns non-zlib error codes before empty messages', () => { + const err = new Error(''); + Object.defineProperty(err, 'code', { + value: 'ERR_SYNC', + }); + + expect(extractErrorMessage(err)).toBe('ERR_SYNC'); + }); + + it('falls back to the Error message', () => { + expect(extractErrorMessage(new Error('direct message'))).toBe('direct message'); + }); + + it('reads a message property from plain objects', () => { + expect(extractErrorMessage({ message: 'object message' })).toBe('object message'); + }); + + it('returns null for unsupported values', () => { + expect(extractErrorMessage(undefined)).toBeNull(); + expect(extractErrorMessage({ message: 123 })).toBeNull(); + }); +}); diff --git a/packages/sync-core/tests/sync-file-prefix.spec.ts b/packages/sync-core/tests/sync-file-prefix.spec.ts new file mode 100644 index 0000000000..a544c58fe0 --- /dev/null +++ b/packages/sync-core/tests/sync-file-prefix.spec.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from 'vitest'; +import { + SyncFilePrefixError, + SyncFilePrefixVersionError, + createSyncFilePrefixHelpers, +} from '../src'; + +describe('createSyncFilePrefixHelpers', () => { + it('formats prefixes with host-supplied prefix and default separator', () => { + const helpers = createSyncFilePrefixHelpers({ prefix: 'pf_' }); + + expect( + helpers.getSyncFilePrefix({ + isCompress: true, + isEncrypt: true, + modelVersion: 17, + }), + ).toBe('pf_CE17__'); + expect( + helpers.getSyncFilePrefix({ + isCompress: false, + isEncrypt: false, + modelVersion: 17, + }), + ).toBe('pf_17__'); + }); + + it('extracts prefix state and leaves the payload untouched', () => { + const helpers = createSyncFilePrefixHelpers({ prefix: 'pf_' }); + + expect(helpers.extractSyncFileStateFromPrefix('pf_CE17__{"task":[]}')).toEqual({ + isCompressed: true, + isEncrypted: true, + modelVersion: 17, + cleanDataStr: '{"task":[]}', + }); + }); + + it('supports decimal model versions for existing sync file compatibility', () => { + const helpers = createSyncFilePrefixHelpers({ prefix: 'pf_' }); + + expect(helpers.extractSyncFileStateFromPrefix('pf_C16.5__{}')).toEqual({ + isCompressed: true, + isEncrypted: false, + modelVersion: 16.5, + cleanDataStr: '{}', + }); + }); + + it('escapes regex characters in host prefix and separator', () => { + const helpers = createSyncFilePrefixHelpers({ + prefix: 'host.sync+', + endSeparator: '.*', + }); + + expect( + helpers.getSyncFilePrefix({ isCompress: true, isEncrypt: false, modelVersion: 1 }), + ).toBe('host.sync+C1.*'); + expect(helpers.extractSyncFileStateFromPrefix('host.sync+C1.*payload')).toEqual({ + isCompressed: true, + isEncrypted: false, + modelVersion: 1, + cleanDataStr: 'payload', + }); + }); + + it('uses the host invalid-prefix error factory when supplied', () => { + class HostInvalidPrefixError extends Error { + override name = 'HostInvalidPrefixError'; + } + let receivedDetails: unknown; + + const helpers = createSyncFilePrefixHelpers({ + prefix: 'pf_', + createInvalidPrefixError: (details) => { + receivedDetails = details; + return new HostInvalidPrefixError(`invalid length ${details.inputLength}`); + }, + }); + + expect(() => helpers.extractSyncFileStateFromPrefix('bad secret payload')).toThrow( + HostInvalidPrefixError, + ); + expect(receivedDetails).toEqual({ + expectedPrefix: 'pf_', + endSeparator: '__', + inputLength: 'bad secret payload'.length, + }); + }); + + it('throws a generic package error without a host error factory', () => { + const helpers = createSyncFilePrefixHelpers({ prefix: 'pf_' }); + + expect(() => helpers.extractSyncFileStateFromPrefix('bad')).toThrow( + SyncFilePrefixError, + ); + }); + + it('rejects formatted model versions that the parser cannot read back', () => { + const helpers = createSyncFilePrefixHelpers({ prefix: 'pf_' }); + + for (const modelVersion of [Number.NaN, Number.POSITIVE_INFINITY, -1, 1e21]) { + expect(() => + helpers.getSyncFilePrefix({ + isCompress: false, + isEncrypt: false, + modelVersion, + }), + ).toThrow(SyncFilePrefixVersionError); + } + }); + + it('rejects parsed model versions that overflow to Infinity', () => { + const helpers = createSyncFilePrefixHelpers({ prefix: 'pf_' }); + + expect(() => + helpers.extractSyncFileStateFromPrefix(`pf_${'9'.repeat(400)}__{}`), + ).toThrow(SyncFilePrefixVersionError); + }); + + it('bounds rejected model-version text in error messages', () => { + const helpers = createSyncFilePrefixHelpers({ prefix: 'pf_' }); + + try { + helpers.extractSyncFileStateFromPrefix(`pf_${'9'.repeat(400)}__{}`); + throw new Error('Expected invalid model version to throw'); + } catch (error) { + expect(error).toBeInstanceOf(SyncFilePrefixVersionError); + expect((error as Error).message.length).toBeLessThan(100); + } + }); +}); diff --git a/src/app/op-log/core/errors/sync-errors.ts b/src/app/op-log/core/errors/sync-errors.ts index e84b22bf45..e218355c50 100644 --- a/src/app/op-log/core/errors/sync-errors.ts +++ b/src/app/op-log/core/errors/sync-errors.ts @@ -1,55 +1,15 @@ import { IValidation } from 'typia'; +import type { SyncFilePrefixInvalidPrefixDetails } from '@sp/sync-core'; +import { extractErrorMessage as extractGenericErrorMessage } from '@sp/sync-core'; import { OpLog } from '../../../core/log'; import { FILE_BASED_SYNC_CONSTANTS } from '../../sync-providers/file-based/file-based-sync.types'; -/** - * Extracts a meaningful error message from various error shapes. - * Handles Error objects with nested cause, zlib error codes, and plain strings. - * This is important because some browser APIs (like DecompressionStream) throw - * errors with empty messages but contain the real error in the 'cause' property. - */ export const extractErrorMessage = (err: unknown): string | null => { - if (typeof err === 'string' && err.length > 0) { - return err; + const message = extractGenericErrorMessage(err); + if (typeof message === 'string' && message.startsWith('Z_')) { + return `Compression error: ${message.replace('Z_', '').replace(/_/g, ' ').toLowerCase()}`; } - - if (err instanceof Error) { - // Check for nested cause first (e.g., DecompressionStream errors) - const cause = (err as Error & { cause?: unknown }).cause; - if (cause instanceof Error && cause.message) { - return cause.message; - } - - // Check for error code (e.g., zlib errors like Z_DATA_ERROR) - const code = (err as Error & { code?: string }).code; - if (typeof code === 'string' && code.length > 0) { - // Make zlib error codes more readable - if (code.startsWith('Z_')) { - return `Compression error: ${code.replace('Z_', '').replace(/_/g, ' ').toLowerCase()}`; - } - return code; - } - - // Use the error's own message if available - if (err.message && err.message.length > 0) { - return err.message; - } - } - - // For objects with message property - if ( - err !== null && - typeof err === 'object' && - 'message' in err && - typeof (err as { message: unknown }).message === 'string' - ) { - const msg = (err as { message: string }).message; - if (msg.length > 0) { - return msg; - } - } - - return null; + return message; }; /** @@ -563,6 +523,15 @@ export class ModelVersionToImportNewerThanLocalError extends AdditionalLogErrorB export class InvalidFilePrefixError extends AdditionalLogErrorBase { override name = 'InvalidFilePrefixError'; + + constructor(details: SyncFilePrefixInvalidPrefixDetails) { + super({ + message: `Invalid sync file prefix. Expected prefix "${details.expectedPrefix}".`, + expectedPrefix: details.expectedPrefix, + endSeparator: details.endSeparator, + inputLength: details.inputLength, + }); + } } export class DataRepairNotPossibleError extends AdditionalLogErrorBase { diff --git a/src/app/op-log/encryption/encrypt-and-compress-handler.service.spec.ts b/src/app/op-log/encryption/encrypt-and-compress-handler.service.spec.ts index 6f6243920a..b81edb2739 100644 --- a/src/app/op-log/encryption/encrypt-and-compress-handler.service.spec.ts +++ b/src/app/op-log/encryption/encrypt-and-compress-handler.service.spec.ts @@ -1,6 +1,6 @@ import { OpLog } from '../../core/log'; import type { SyncLogger } from '@sp/sync-core'; -import { JsonParseError } from '../core/errors/sync-errors'; +import { extractErrorMessage, JsonParseError } from '../core/errors/sync-errors'; import { EncryptAndCompressHandlerService } from './encrypt-and-compress-handler.service'; import { getErrorTxt } from '../../util/get-error-text'; @@ -140,6 +140,34 @@ describe('EncryptAndCompressHandlerService', () => { expect(result.data).toEqual(complexData); expect(result.modelVersion).toBe(2); }); + + it('should round-trip compressed unencrypted sync data', async () => { + const testData = { + tasks: Array.from({ length: 20 }, (_, i) => ({ + id: `task-${i}`, + title: `Task ${i}`, + })), + }; + + const compressed = await service.compressAndEncrypt({ + data: testData, + modelVersion: 3, + isCompress: true, + isEncrypt: false, + }); + + expect(compressed.startsWith('pf_C3__')).toBeTrue(); + + const result = await service.decompressAndDecrypt({ + dataStr: compressed, + encryptKey: undefined, + }); + + expect(result).toEqual({ + data: testData, + modelVersion: 3, + }); + }); }); }); @@ -233,3 +261,14 @@ describe('JsonParseError', () => { expect(error.dataSample).toBeDefined(); }); }); + +describe('extractErrorMessage', () => { + it('preserves app-side compression error code wording', () => { + const error = new Error(''); + Object.defineProperty(error, 'code', { + value: 'Z_BUF_ERROR', + }); + + expect(extractErrorMessage(error)).toBe('Compression error: buf error'); + }); +}); diff --git a/src/app/op-log/util/sync-file-prefix.spec.ts b/src/app/op-log/util/sync-file-prefix.spec.ts new file mode 100644 index 0000000000..0af0233c32 --- /dev/null +++ b/src/app/op-log/util/sync-file-prefix.spec.ts @@ -0,0 +1,22 @@ +import { OpLog } from '../../core/log'; +import { InvalidFilePrefixError } from '../core/errors/sync-errors'; +import { extractSyncFileStateFromPrefix } from './sync-file-prefix'; + +describe('sync-file-prefix app shim', () => { + beforeEach(() => { + spyOn(OpLog, 'log').and.stub(); + }); + + it('throws InvalidFilePrefixError without logging raw sync payload content', () => { + const rawPayload = '{"task":{"entities":{"task1":{"title":"secret task"}}}}'; + + expect(() => extractSyncFileStateFromPrefix(rawPayload)).toThrowError( + InvalidFilePrefixError, + ); + + const logText = (OpLog.log as jasmine.Spy).calls.allArgs().flat().join('\n'); + expect(logText).toContain('inputLength'); + expect(logText).not.toContain('secret task'); + expect(logText).not.toContain(rawPayload); + }); +}); diff --git a/src/app/op-log/util/sync-file-prefix.ts b/src/app/op-log/util/sync-file-prefix.ts index 7a60d4f0dc..7d5aaae821 100644 --- a/src/app/op-log/util/sync-file-prefix.ts +++ b/src/app/op-log/util/sync-file-prefix.ts @@ -1,42 +1,16 @@ +import { createSyncFilePrefixHelpers } from '@sp/sync-core'; +import type { SyncFilePrefixParams, SyncFilePrefixParamsOutput } from '@sp/sync-core'; import { REMOTE_FILE_CONTENT_PREFIX } from '../sync-providers/provider.const'; import { InvalidFilePrefixError } from '../core/errors/sync-errors'; -const PREFIX = REMOTE_FILE_CONTENT_PREFIX; -const END_SEPERATOR = '__'; +export type { SyncFilePrefixParams, SyncFilePrefixParamsOutput }; -export interface SyncFilePrefixParams { - isCompress: boolean; - isEncrypt: boolean; - modelVersion: number; -} -export interface SyncFilePrefixParamsOutput { - isCompressed: boolean; - isEncrypted: boolean; - modelVersion: number; - cleanDataStr: string; -} +const syncFilePrefixHelpers = createSyncFilePrefixHelpers({ + prefix: REMOTE_FILE_CONTENT_PREFIX, + createInvalidPrefixError: (details): Error => new InvalidFilePrefixError(details), +}); -export const getSyncFilePrefix = (cfg: SyncFilePrefixParams): string => { - const c = cfg.isCompress ? 'C' : ''; - const e = cfg.isEncrypt ? 'E' : ''; - return `${PREFIX}${c}${e}${cfg.modelVersion}${END_SEPERATOR}`; -}; +export const getSyncFilePrefix = syncFilePrefixHelpers.getSyncFilePrefix; -export const extractSyncFileStateFromPrefix = ( - dataStr: string, -): SyncFilePrefixParamsOutput => { - // Modified regex to match decimal numbers - const match = dataStr.match( - new RegExp(`^${PREFIX}(C)?(E)?(\\d+(?:\\.\\d+)?)${END_SEPERATOR}`), - ); - if (!match) { - throw new InvalidFilePrefixError(dataStr); - } - - return { - isCompressed: !!match[1], - isEncrypted: !!match[2], - modelVersion: parseFloat(match[3]), // Changed to parseFloat - cleanDataStr: dataStr.slice(match[0].length), - }; -}; +export const extractSyncFileStateFromPrefix = + syncFilePrefixHelpers.extractSyncFileStateFromPrefix;