From ec41c1ce382003adee236f316884d1c8489a8409 Mon Sep 17 00:00:00 2001 From: Convly Date: Fri, 13 Jan 2023 16:43:20 +0100 Subject: [PATCH 01/16] Init diagnostics --- .../data-transfer/src/engine/diagnostic.ts | 119 ++++++++ .../core/data-transfer/src/engine/errors.ts | 33 +++ .../core/data-transfer/src/engine/index.ts | 275 +++++++++++++++--- .../src/engine/validation/index.ts | 3 +- .../src/engine/validation/provider.ts | 27 ++ .../core/data-transfer/src/errors/base.ts | 19 ++ .../data-transfer/src/errors/constants.ts | 6 + .../core/data-transfer/src/errors/index.ts | 2 + .../data-transfer/types/transfer-engine.d.ts | 23 +- .../strapi/lib/commands/transfer/export.js | 27 +- .../strapi/lib/commands/transfer/import.js | 28 +- 11 files changed, 507 insertions(+), 55 deletions(-) create mode 100644 packages/core/data-transfer/src/engine/diagnostic.ts create mode 100644 packages/core/data-transfer/src/engine/errors.ts create mode 100644 packages/core/data-transfer/src/engine/validation/provider.ts create mode 100644 packages/core/data-transfer/src/errors/base.ts create mode 100644 packages/core/data-transfer/src/errors/constants.ts create mode 100644 packages/core/data-transfer/src/errors/index.ts diff --git a/packages/core/data-transfer/src/engine/diagnostic.ts b/packages/core/data-transfer/src/engine/diagnostic.ts new file mode 100644 index 0000000000..4ea31ceb2d --- /dev/null +++ b/packages/core/data-transfer/src/engine/diagnostic.ts @@ -0,0 +1,119 @@ +import { EventEmitter } from 'events'; + +export interface IDiagnosticReporterOptions { + stackSize?: number; +} + +export type GenericDiagnostic = { + kind: K; + details: { + message: string; + createdAt: Date; + } & T; +}; + +export type DiagnosticKind = 'error' | 'warning' | 'info'; + +export type DiagnosticListener = ( + diagnostic: { kind: T } & Diagnostic extends infer U ? U : 'foo' +) => void | Promise; + +export type DiagnosticEvent = 'diagnostic' | `diagnostic.${DiagnosticKind}`; + +export type GetEventListener = E extends 'diagnostic' + ? DiagnosticListener + : E extends `diagnostic.${infer K}` + ? K extends DiagnosticKind + ? DiagnosticListener + : never + : never; + +export type Diagnostic = ErrorDiagnostic | WarningDiagnostic | InfoDiagnostic; + +export type ErrorDiagnosticSeverity = 'fatal' | 'error' | 'silly'; + +export type ErrorDiagnostic = GenericDiagnostic< + 'error', + { + name: string; + severity: ErrorDiagnosticSeverity; + error: Error; + } +>; + +export type WarningDiagnostic = GenericDiagnostic< + 'warning', + { + origin?: string; + } +>; + +export type InfoDiagnostic = GenericDiagnostic< + 'info', + { + params?: T; + } +>; + +export interface IDiagnosticReporter { + stack: { + readonly size: number; + readonly items: Diagnostic[]; + }; + + report(diagnostic: Diagnostic): IDiagnosticReporter; + onDiagnostic(listener: DiagnosticListener): IDiagnosticReporter; + on(kind: T, listener: DiagnosticListener): IDiagnosticReporter; +} + +const createDiagnosticReporter = ( + options: IDiagnosticReporterOptions = {} +): IDiagnosticReporter => { + const { stackSize = -1 } = options; + + const emitter = new EventEmitter(); + const stack: Diagnostic[] = []; + + const addListener = (event: T, listener: GetEventListener) => { + emitter.on(event, listener); + }; + + return { + stack: { + get size() { + return stack.length; + }, + + get items() { + return stack; + }, + }, + + report(diagnostic: Diagnostic) { + emitter.emit('diagnostic', diagnostic); + emitter.emit(`diagnostic.${diagnostic.kind}`, diagnostic); + + if (stackSize !== -1 && stack.length >= stackSize) { + stack.shift(); + } + + stack.push(diagnostic); + + return this; + }, + + onDiagnostic(listener: DiagnosticListener) { + addListener('diagnostic', listener); + + return this; + }, + + on(kind: T, listener: DiagnosticListener) { + addListener(`diagnostic.${kind}`, listener as never); + + return this; + }, + }; +}; + +export { createDiagnosticReporter }; diff --git a/packages/core/data-transfer/src/engine/errors.ts b/packages/core/data-transfer/src/engine/errors.ts new file mode 100644 index 0000000000..aedd1bfd47 --- /dev/null +++ b/packages/core/data-transfer/src/engine/errors.ts @@ -0,0 +1,33 @@ +import { DataTransferError, Severity, SeverityKind } from '../errors'; + +type TransferEngineStep = 'initialization' | 'validation' | 'transfer'; + +type TransferEngineErrorDetails

= { + step: P; +} & ([U] extends [never] ? unknown : { details?: U }); + +class TransferEngineError< + P extends TransferEngineStep = TransferEngineStep, + U = never, + T extends TransferEngineErrorDetails = TransferEngineErrorDetails +> extends DataTransferError { + constructor(severity: Severity, message?: string, details?: T | null) { + super('engine', severity, message, details); + } +} + +class TransferEngineInitializationError extends TransferEngineError<'initialization'> { + constructor(message?: string) { + super(SeverityKind.FATAL, message, { step: 'initialization' }); + } +} + +class TransferEngineValidationError< + T extends { check: string } = { check: string } +> extends TransferEngineError<'validation', T> { + constructor(message?: string, details?: T) { + super(SeverityKind.FATAL, message, { step: 'validation', details }); + } +} + +export { TransferEngineError, TransferEngineInitializationError, TransferEngineValidationError }; diff --git a/packages/core/data-transfer/src/engine/index.ts b/packages/core/data-transfer/src/engine/index.ts index f26706a4e0..14f15e811d 100644 --- a/packages/core/data-transfer/src/engine/index.ts +++ b/packages/core/data-transfer/src/engine/index.ts @@ -1,6 +1,7 @@ import { PassThrough, Transform, Readable, Writable, Stream } from 'stream'; import { extname } from 'path'; -import { isEmpty, uniq } from 'lodash/fp'; +import { EOL } from 'os'; +import { isEmpty, uniq, last } from 'lodash/fp'; import { diff as semverDiff } from 'semver'; import type { Schema } from '@strapi/strapi'; @@ -16,12 +17,20 @@ import type { ITransferResults, TransferStage, TransferTransform, + IProvider, } from '../../types'; import type { Diff } from '../utils/json'; -import { compareSchemas } from './validation/schemas'; +import { compareSchemas, validateProvider } from './validation'; import { filter, map } from '../utils/stream'; +import { TransferEngineValidationError } from './errors'; +import { + createDiagnosticReporter, + IDiagnosticReporter, + ErrorDiagnosticSeverity, +} from './diagnostic'; + export const TRANSFER_STAGES: ReadonlyArray = Object.freeze([ 'entities', 'links', @@ -53,17 +62,13 @@ class TransferEngine< stream: PassThrough; }; - constructor( - sourceProvider: ISourceProvider, - destinationProvider: IDestinationProvider, - options: ITransferEngineOptions - ) { - if (sourceProvider.type !== 'source') { - throw new Error("SourceProvider does not have type 'source'"); - } - if (destinationProvider.type !== 'destination') { - throw new Error("DestinationProvider does not have type 'destination'"); - } + diagnostics: IDiagnosticReporter; + + constructor(sourceProvider: S, destinationProvider: D, options: ITransferEngineOptions) { + this.diagnostics = createDiagnosticReporter(); + + validateProvider('source', sourceProvider); + this.sourceProvider = sourceProvider; this.destinationProvider = destinationProvider; this.options = options; @@ -71,6 +76,56 @@ class TransferEngine< this.progress = { data: {}, stream: new PassThrough({ objectMode: true }) }; } + /** + * Report a fatal error and throw it + */ + #panic(error: Error) { + this.#reportError(error, 'fatal'); + + throw error; + } + + /** + * Report an error diagnostic + */ + #reportError(error: Error, severity: ErrorDiagnosticSeverity) { + this.diagnostics.report({ + kind: 'error', + details: { + severity, + createdAt: new Date(), + name: error.name, + message: error.message, + error, + }, + }); + } + + /** + * Report a warning diagnostic + */ + #reportWarning(message: string, origin?: string) { + this.diagnostics.report({ + kind: 'warning', + details: { createdAt: new Date(), message, origin }, + }); + } + + /** + * Report an info diagnostic + */ + #reportInfo(message: string, params?: unknown) { + this.diagnostics.report({ + kind: 'info', + details: { createdAt: new Date(), message, params }, + }); + } + + /** + * Create and return a transform stream based on the given stage and options. + * + * Allowed transformations includes 'filter' and 'map'. + */ #createStageTransformStream( key: T, options: { includeGlobal?: boolean } = {} @@ -101,6 +156,11 @@ class TransferEngine< return stream; } + /** + * Update the Engine's transfer progress data for a given stage. + * + * Providing aggregate options enable custom computation to get the size (bytes) or the aggregate key associated with the data + */ #updateTransferProgress( stage: TransferStage, data: T, @@ -142,6 +202,11 @@ class TransferEngine< } } + /** + * Create and return a PassThrough stream. + * + * Upon writing data into it, it'll update the Engine's transfer progress data and trigger stage update events. + */ #progressTracker( stage: TransferStage, aggregate?: { @@ -159,10 +224,16 @@ class TransferEngine< }); } + /** + * Shorthand method used to trigger transfer update events to every listeners + */ #emitTransferUpdate(type: 'init' | 'start' | 'finish' | 'error', payload?: object) { this.progress.stream.emit(`transfer::${type}`, payload); } + /** + * Shorthand method used to trigger stage update events to every listeners + */ #emitStageUpdate(type: 'start' | 'finish' | 'progress' | 'skip', transferStage: TransferStage) { this.progress.stream.emit(`stage::${type}`, { data: this.progress.data, @@ -170,9 +241,25 @@ class TransferEngine< }); } + /** + * Run a version check between two strapi version (source and destination) using the strategy given to the engine during initialization. + * + * If there is a mismatch, throws a validation error. + */ #assertStrapiVersionIntegrity(sourceVersion?: string, destinationVersion?: string) { const strategy = this.options.versionStrategy || DEFAULT_VERSION_STRATEGY; + const reject = () => { + throw new TransferEngineValidationError( + `The source and destination provide are targeting incompatible Strapi versions (using the "${strategy}" strategy). The source (${this.sourceProvider.name}) version is ${sourceVersion} and the destination (${this.destinationProvider.name}) version is ${destinationVersion}`, + { + check: 'strapi.version', + strategy, + versions: { source: sourceVersion, destination: destinationVersion }, + } + ); + }; + if ( !sourceVersion || !destinationVersion || @@ -185,11 +272,10 @@ class TransferEngine< let diff; try { diff = semverDiff(sourceVersion, destinationVersion); - } catch (e: unknown) { - throw new Error( - `Strapi versions doesn't match (${strategy} check): ${sourceVersion} does not match with ${destinationVersion}` - ); + } catch { + reject(); } + if (!diff) { return; } @@ -207,13 +293,17 @@ class TransferEngine< return; } - throw new Error( - `Strapi versions doesn't match (${strategy} check): ${sourceVersion} does not match with ${destinationVersion}` - ); + reject(); } + /** + * Run a check between two set of schemas (source and destination) using the strategy given to the engine during initialization. + * + * If there are differences and/or incompatibilities between source and destination schemas, then throw a validation error. + */ #assertSchemasMatching(sourceSchemas: SchemaMap, destinationSchemas: SchemaMap) { const strategy = this.options.schemaStrategy || DEFAULT_SCHEMA_STRATEGY; + if (strategy === 'ignore') { return; } @@ -232,14 +322,48 @@ class TransferEngine< }); if (!isEmpty(diffs)) { - throw new Error( - `Import process failed because the project doesn't have a matching data structure - ${JSON.stringify(diffs, null, 2)} - ` + const formattedDiffs = Object.entries(diffs) + .map(([uid, ctDiffs]) => { + let msg = `- ${uid}:${EOL}`; + + msg += ctDiffs + .sort((a, b) => (a.kind > b.kind ? -1 : 1)) + .map((diff) => { + if (diff.kind === 'added') { + return `Added "${diff.path}": "${diff.value}" (${diff.type})`; + } + + if (diff.kind === 'deleted') { + return `Removed "${diff.path}"`; + } + + if (diff.kind === 'modified') { + return `Modified "${diff.path}". "${diff.values[0]}" (${diff.types[0]}) => "${diff.values[1]}" (${diff.types[1]})`; + } + + throw new Error(`Invalid diff found for "${uid}"`); + }) + .map((line) => ` - ${line}`) + .join(EOL); + + return msg; + }) + .join(EOL); + + throw new TransferEngineValidationError( + `Invalid schema changes detected during integrity checks (using the ${strategy} strategy). Please find a summary of the changes below:\n${formattedDiffs}`, + { + check: 'schema.changes', + strategy, + diffs, + } ); } } + /** + * Build a run a stage transfer based on the given parameters. + */ async #transferStage(options: { stage: TransferStage; source?: Readable; @@ -251,7 +375,7 @@ class TransferEngine< if (!source || !destination) { // Wait until source and destination are closed - await Promise.allSettled( + const results = await Promise.allSettled( [source, destination].map((stream) => { // if stream is undefined or already closed, resolve immediately if (!stream || stream.destroyed) { @@ -265,6 +389,12 @@ class TransferEngine< }) ); + results.forEach((state) => { + if (state.status === 'rejected') { + this.#reportWarning(state.reason, `transfer(${stage})`); + } + }); + this.#emitStageUpdate('skip', stage); return; @@ -283,7 +413,15 @@ class TransferEngine< stream = stream.pipe(tracker); } - stream.pipe(destination).on('error', reject).on('close', resolve); + stream + .pipe(destination) + .on('error', (e) => { + // TODO ? + // this.#reportError(e, 'error'); + // destination.destroy(e); + reject(e); + }) + .on('close', resolve); }); this.#emitStageUpdate('finish', stage); @@ -302,12 +440,36 @@ class TransferEngine< } } + /** + * Run the bootstrap method in both source and destination providers + */ async bootstrap(): Promise { - await Promise.all([this.sourceProvider.bootstrap?.(), this.destinationProvider.bootstrap?.()]); + const results = await Promise.allSettled([ + this.sourceProvider.bootstrap?.(), + this.destinationProvider.bootstrap?.(), + ]); + + results.forEach((result) => { + if (result.status === 'rejected') { + this.#panic(result.reason); + } + }); } + /** + * Run the close method in both source and destination providers + */ async close(): Promise { - await Promise.all([this.sourceProvider.close?.(), this.destinationProvider.close?.()]); + const results = await Promise.allSettled([ + this.sourceProvider.close?.(), + this.destinationProvider.bootstrap?.(), + ]); + + results.forEach((result) => { + if (result.status === 'rejected') { + this.#panic(result.reason); + } + }); } async #resolveProviderResource() { @@ -323,7 +485,7 @@ class TransferEngine< } } - async integrityCheck(): Promise { + async integrityCheck() { try { const sourceMetadata = await this.sourceProvider.getMetadata(); const destinationMetadata = await this.destinationProvider.getMetadata(); @@ -341,10 +503,12 @@ class TransferEngine< if (sourceSchemas && destinationSchemas) { this.#assertSchemasMatching(sourceSchemas, destinationSchemas); } - - return true; } catch (error) { - return false; + if (error instanceof Error) { + this.#panic(error); + } + + throw error; } } @@ -356,17 +520,13 @@ class TransferEngine< this.#emitTransferUpdate('init'); await this.bootstrap(); await this.init(); - const isValidTransfer = await this.integrityCheck(); - if (!isValidTransfer) { - // TODO: provide the log from the integrity check - throw new Error( - `Unable to transfer the data between ${this.sourceProvider.name} and ${this.destinationProvider.name}.\nPlease refer to the log above for more information.` - ); - } + + await this.integrityCheck(); this.#emitTransferUpdate('start'); await this.beforeTransfer(); + // Run the transfer stages await this.transferSchemas(); await this.transferEntities(); @@ -380,9 +540,20 @@ class TransferEngine< } catch (e: unknown) { this.#emitTransferUpdate('error', { error: e }); + const lastDiagnostic = last(this.diagnostics.stack.items); + + // Do not report an error diagnostic if the last one reported the same error + if ( + e instanceof Error && + (!lastDiagnostic || lastDiagnostic.kind !== 'error' || lastDiagnostic.details.error !== e) + ) { + this.#reportError(e, 'fatal'); + } + // Rollback the destination provider if an exception is thrown during the transfer // Note: This will be configurable in the future await this.destinationProvider.rollback?.(e as Error); + throw e; } @@ -394,8 +565,23 @@ class TransferEngine< } async beforeTransfer(): Promise { - await this.sourceProvider.beforeTransfer?.(); - await this.destinationProvider.beforeTransfer?.(); + const runWithDiagnostic = async (provider: IProvider) => { + try { + await provider.beforeTransfer?.(); + } catch (error) { + // Error happening during the before transfer step should be considered fatal errors + if (error instanceof Error) { + this.#panic(error); + } else { + this.#panic( + new Error(`Unknwon error when executing "beforeTransfer" on the ${origin} provider`) + ); + } + } + }; + + await runWithDiagnostic(this.sourceProvider); + await runWithDiagnostic(this.destinationProvider); } async transferSchemas(): Promise { @@ -443,7 +629,7 @@ class TransferEngine< const transform = this.#createStageTransformStream(stage); const tracker = this.#progressTracker(stage, { size: (value: IAsset) => value.stats.size, - key: (value: IAsset) => extname(value.filename), + key: (value: IAsset) => extname(value.filename) ?? 'NA', }); await this.#transferStage({ stage, source, destination, transform, tracker }); @@ -462,10 +648,7 @@ class TransferEngine< } } -export const createTransferEngine = < - S extends ISourceProvider = ISourceProvider, - D extends IDestinationProvider = IDestinationProvider ->( +export const createTransferEngine = ( sourceProvider: S, destinationProvider: D, options: ITransferEngineOptions diff --git a/packages/core/data-transfer/src/engine/validation/index.ts b/packages/core/data-transfer/src/engine/validation/index.ts index 197f1788ef..9fc174d463 100644 --- a/packages/core/data-transfer/src/engine/validation/index.ts +++ b/packages/core/data-transfer/src/engine/validation/index.ts @@ -1 +1,2 @@ -export * as schemas from './schemas'; +export * from './schemas'; +export * from './provider'; diff --git a/packages/core/data-transfer/src/engine/validation/provider.ts b/packages/core/data-transfer/src/engine/validation/provider.ts new file mode 100644 index 0000000000..b5d95eeece --- /dev/null +++ b/packages/core/data-transfer/src/engine/validation/provider.ts @@ -0,0 +1,27 @@ +import { capitalize } from 'lodash/fp'; + +import type { IDestinationProvider, ISourceProvider, ProviderType } from '../../../types'; +import { TransferEngineValidationError } from '../errors'; + +const reject = (reason: string): never => { + throw new TransferEngineValidationError(`Invalid provider supplied. ${reason}`); +}; + +const validateProvider = ( + type: ProviderType, + provider?: ([T] extends ['source'] ? ISourceProvider : IDestinationProvider) | null +) => { + if (!provider) { + return reject( + `Expected an instance of "${capitalize(type)}Provider", but got "${typeof provider}" instead.` + ); + } + + if (provider.type !== type) { + return reject( + `Expected the provider to be of type "${type}" but got "${provider.type}" instead.` + ); + } +}; + +export { validateProvider }; diff --git a/packages/core/data-transfer/src/errors/base.ts b/packages/core/data-transfer/src/errors/base.ts new file mode 100644 index 0000000000..1d3987a39f --- /dev/null +++ b/packages/core/data-transfer/src/errors/base.ts @@ -0,0 +1,19 @@ +import { Severity } from './constants'; + +class DataTransferError extends Error { + origin: string; + + severity: Severity; + + details: T | null; + + constructor(origin: string, severity: Severity, message?: string, details?: T | null) { + super(message); + + this.origin = origin; + this.severity = severity; + this.details = details ?? null; + } +} + +export { DataTransferError }; diff --git a/packages/core/data-transfer/src/errors/constants.ts b/packages/core/data-transfer/src/errors/constants.ts new file mode 100644 index 0000000000..6c3964cc3f --- /dev/null +++ b/packages/core/data-transfer/src/errors/constants.ts @@ -0,0 +1,6 @@ +export const SeverityKind = { + FATAL: 1, + ERROR: 2, + SILLY: 3, +} as const; +export type Severity = typeof SeverityKind[keyof typeof SeverityKind]; diff --git a/packages/core/data-transfer/src/errors/index.ts b/packages/core/data-transfer/src/errors/index.ts new file mode 100644 index 0000000000..41bf5eaffb --- /dev/null +++ b/packages/core/data-transfer/src/errors/index.ts @@ -0,0 +1,2 @@ +export * from './constants'; +export * from './base'; diff --git a/packages/core/data-transfer/types/transfer-engine.d.ts b/packages/core/data-transfer/types/transfer-engine.d.ts index bbf6d588bc..cec7ae9f00 100644 --- a/packages/core/data-transfer/types/transfer-engine.d.ts +++ b/packages/core/data-transfer/types/transfer-engine.d.ts @@ -1,7 +1,10 @@ -import type { IAsset, IEntity, ILink } from './common-entities'; -import type { ITransferResults, TransferTransform } from './utils'; -import type { ISourceProvider, IDestinationProvider } from './providers'; +import { PassThrough } from 'stream'; import type { Schema } from '@strapi/strapi'; +import type { IAsset, IEntity, ILink } from './common-entities'; +import type { ITransferResults, TransferTransform, TransferProgress } from './utils'; +import type { ISourceProvider, IDestinationProvider } from './providers'; +import type { Severity } from '../src/errors'; +import type { DiagnosticReporter } from '../src/engine/diagnostic'; /** * Defines the capabilities and properties of the transfer engine @@ -22,6 +25,18 @@ export interface ITransferEngine< * The options used to customize the behavio of the transfer engine */ options: ITransferEngineOptions; + /** + * A diagnostic reporter instance used to gather information about + * errors, warnings and information emitted by the engine + */ + diagnostics: DiagnosticReporter; + /** + * Utilities used to retrieve transfer progress data + */ + progress: { + data: TransferProgress; + stream: PassThrough; + }; /** * Runs the integrity check which will make sure it's possible @@ -29,7 +44,7 @@ export interface ITransferEngine< * * Note: It requires to read the content of the source & destination metadata files */ - integrityCheck(): Promise; + integrityCheck(): Promise; /** * Start streaming selected data from the source to the destination diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index be5d33eb08..fc5029d0e9 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -74,6 +74,29 @@ module.exports = async (opts) => { }, }); + engine.diagnostics + .on('error', ({ details }) => { + const { createdAt, severity, name, message } = details; + + logger.error( + chalk.red(`[${createdAt.toISOString()}] [error] (${severity}) ${name}: ${message}`) + ); + }) + .on('info', ({ details }) => { + const { createdAt, message, params } = details; + + const msg = typeof message === 'function' ? message(params) : message; + + logger.info(chalk.blue(`[${createdAt.toISOString()}] [info] ${msg}`)); + }) + .on('warning', ({ details }) => { + const { createdAt, origin, message } = details; + + logger.warn( + chalk.yellow(`[${createdAt.toISOString()}] [warning] (${origin ?? 'transfer'}) ${message}`) + ); + }); + const progress = engine.progress.stream; const getTelemetryPayload = (/* payload */) => { @@ -104,9 +127,9 @@ module.exports = async (opts) => { logger.log(`${chalk.bold('Export process has been completed successfully!')}`); logger.log(`Export archive is in ${chalk.green(outFile)}`); - } catch (e) { + } catch { await strapi.telemetry.send('didDEITSProcessFail', getTelemetryPayload()); - logger.error('Export process failed unexpectedly:', e.toString()); + logger.error('Export process failed'); process.exit(1); } diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index 72fd378d79..61e36b228b 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -14,6 +14,7 @@ const { const { isObject } = require('lodash/fp'); const path = require('path'); +const chalk = require('chalk'); const strapi = require('../../index'); const { buildTransferTable, DEFAULT_IGNORED_CONTENT_TYPES } = require('./utils'); @@ -83,6 +84,29 @@ module.exports = async (opts) => { const engine = createTransferEngine(source, destination, engineOptions); + engine.diagnostics + .on('error', ({ details }) => { + const { createdAt, severity, name, message } = details; + + logger.error( + chalk.red(`[${createdAt.toISOString()}] [error] (${severity}) ${name}: ${message}`) + ); + }) + .on('info', ({ details }) => { + const { createdAt, message, params } = details; + + const msg = typeof message === 'function' ? message(params) : message; + + logger.info(chalk.blue(`[${createdAt.toISOString()}] [info] ${msg}`)); + }) + .on('warning', ({ details }) => { + const { createdAt, origin, message } = details; + + logger.warn( + chalk.yellow(`[${createdAt.toISOString()}] [warning] (${origin ?? 'transfer'}) ${message}`) + ); + }); + const progress = engine.progress.stream; const getTelemetryPayload = () => { return { @@ -106,8 +130,8 @@ module.exports = async (opts) => { logger.info('Import process has been completed successfully!'); } catch (e) { await strapiInstance.telemetry.send('didDEITSProcessFail', getTelemetryPayload()); - logger.error('Import process failed unexpectedly:'); - logger.error(e); + logger.error('Import process failed'); + process.exit(1); } From 51b4848540f1f0b66433c6d38cbe8e27546531a7 Mon Sep 17 00:00:00 2001 From: Bassel Date: Tue, 17 Jan 2023 14:30:39 +0200 Subject: [PATCH 02/16] validate destination provider --- packages/core/data-transfer/src/engine/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/data-transfer/src/engine/index.ts b/packages/core/data-transfer/src/engine/index.ts index 14f15e811d..b17de93b76 100644 --- a/packages/core/data-transfer/src/engine/index.ts +++ b/packages/core/data-transfer/src/engine/index.ts @@ -68,6 +68,7 @@ class TransferEngine< this.diagnostics = createDiagnosticReporter(); validateProvider('source', sourceProvider); + validateProvider('destination', destinationProvider); this.sourceProvider = sourceProvider; this.destinationProvider = destinationProvider; From 8f2276f07279fa87dbff0ab18538d8a198f678d6 Mon Sep 17 00:00:00 2001 From: Bassel Date: Tue, 17 Jan 2023 14:46:33 +0200 Subject: [PATCH 03/16] fix import --- .../strapi/providers/remote-destination/__tests__/utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/data-transfer/src/strapi/providers/remote-destination/__tests__/utils.test.ts b/packages/core/data-transfer/src/strapi/providers/remote-destination/__tests__/utils.test.ts index 4f60192a56..613d88af31 100644 --- a/packages/core/data-transfer/src/strapi/providers/remote-destination/__tests__/utils.test.ts +++ b/packages/core/data-transfer/src/strapi/providers/remote-destination/__tests__/utils.test.ts @@ -1,5 +1,5 @@ import { WebSocket } from 'ws'; -import { TRANSFER_PATH } from '../../../../../lib/strapi/remote/constants'; +import { TRANSFER_PATH } from '../../../remote/constants'; import { CommandMessage } from '../../../../../types/remote/protocol/client'; import { createDispatcher } from '../utils'; From 9e00d3ba420251876ea2da99a5eec4fd282f4411 Mon Sep 17 00:00:00 2001 From: Bassel Date: Tue, 17 Jan 2023 14:48:35 +0200 Subject: [PATCH 04/16] fixing another import path --- .../core/data-transfer/src/strapi/__tests__/register.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/data-transfer/src/strapi/__tests__/register.test.ts b/packages/core/data-transfer/src/strapi/__tests__/register.test.ts index 9063b60c19..fc5902845a 100644 --- a/packages/core/data-transfer/src/strapi/__tests__/register.test.ts +++ b/packages/core/data-transfer/src/strapi/__tests__/register.test.ts @@ -2,7 +2,7 @@ import { getStrapiFactory } from '../../__tests__/test-utils'; import { createTransferHandler } from '../remote/handlers'; import register from '../register'; -import { TRANSFER_PATH } from '../../../lib/strapi/remote/constants'; +import { TRANSFER_PATH } from '../remote/constants'; afterEach(() => { jest.clearAllMocks(); From fb374b6a00979c33805f5543b54acc72b904c072 Mon Sep 17 00:00:00 2001 From: Bassel Date: Tue, 17 Jan 2023 15:10:00 +0200 Subject: [PATCH 05/16] fix closing --- packages/core/data-transfer/src/engine/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/data-transfer/src/engine/index.ts b/packages/core/data-transfer/src/engine/index.ts index b17de93b76..a46604a983 100644 --- a/packages/core/data-transfer/src/engine/index.ts +++ b/packages/core/data-transfer/src/engine/index.ts @@ -463,7 +463,7 @@ class TransferEngine< async close(): Promise { const results = await Promise.allSettled([ this.sourceProvider.close?.(), - this.destinationProvider.bootstrap?.(), + this.destinationProvider.close?.(), ]); results.forEach((result) => { From 601218fe3ad2a39454af4b63a80f7cf36aa84b56 Mon Sep 17 00:00:00 2001 From: Bassel Date: Tue, 17 Jan 2023 16:17:59 +0200 Subject: [PATCH 06/16] mock diagnostics --- .../strapi/lib/commands/__tests__/data-transfer/export.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/strapi/lib/commands/__tests__/data-transfer/export.test.js b/packages/core/strapi/lib/commands/__tests__/data-transfer/export.test.js index cc04449279..06afadc740 100644 --- a/packages/core/strapi/lib/commands/__tests__/data-transfer/export.test.js +++ b/packages/core/strapi/lib/commands/__tests__/data-transfer/export.test.js @@ -26,6 +26,7 @@ describe('Export', () => { }, sourceProvider: { name: 'testSource' }, destinationProvider: { name: 'testDestination' }, + diagnostics: { on: jest.fn().mockReturnThis() }, }; }, }, From da8135d1c04a8e1e67f4e7d608d21d3440d013df Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Fri, 20 Jan 2023 16:36:10 +0100 Subject: [PATCH 07/16] Replace conventional errors with diagnostics --- .../data-transfer/src/engine/diagnostic.ts | 14 +++++ .../core/data-transfer/src/engine/errors.ts | 15 ++++- .../core/data-transfer/src/engine/index.ts | 10 ++-- .../data-transfer/src/errors/constants.ts | 10 ++-- .../data-transfer/src/errors/providers.ts | 40 ++++++++++++++ .../providers/local-destination/index.ts | 49 +++++++++-------- .../strategies/restore/configuration.ts | 3 +- .../strategies/restore/entities.ts | 3 +- .../strategies/restore/index.ts | 3 +- .../strategies/restore/links.ts | 3 +- .../strapi/providers/local-source/index.ts | 21 ++----- .../providers/remote-destination/index.ts | 29 ++++++++-- .../src/strapi/remote/constants.ts | 1 + .../src/strapi/remote/handlers.ts | 44 +++++++++++---- .../core/data-transfer/src/utils/providers.ts | 15 +++++ .../strapi/lib/commands/transfer/export.js | 27 ++------- .../strapi/lib/commands/transfer/import.js | 25 +-------- .../strapi/lib/commands/transfer/transfer.js | 6 +- .../strapi/lib/commands/utils/formatter.js | 55 +++++++++++++++++++ packages/utils/logger/lib/index.js | 3 +- .../logger/lib/output-file-configuration.js | 24 ++++++++ 21 files changed, 283 insertions(+), 117 deletions(-) create mode 100644 packages/core/data-transfer/src/errors/providers.ts create mode 100644 packages/core/data-transfer/src/utils/providers.ts create mode 100644 packages/core/strapi/lib/commands/utils/formatter.js create mode 100644 packages/utils/logger/lib/output-file-configuration.js diff --git a/packages/core/data-transfer/src/engine/diagnostic.ts b/packages/core/data-transfer/src/engine/diagnostic.ts index 4ea31ceb2d..d4dc193fac 100644 --- a/packages/core/data-transfer/src/engine/diagnostic.ts +++ b/packages/core/data-transfer/src/engine/diagnostic.ts @@ -78,6 +78,16 @@ const createDiagnosticReporter = ( emitter.on(event, listener); }; + const isDiagnosticValid = (diagnostic: Diagnostic) => { + if (!diagnostic.kind || !diagnostic.details) { + return false; + } + if (!diagnostic.details.message) { + return false; + } + return true; + }; + return { stack: { get size() { @@ -90,6 +100,10 @@ const createDiagnosticReporter = ( }, report(diagnostic: Diagnostic) { + if (!isDiagnosticValid(diagnostic)) { + return this; + } + emitter.emit('diagnostic', diagnostic); emitter.emit(`diagnostic.${diagnostic.kind}`, diagnostic); diff --git a/packages/core/data-transfer/src/engine/errors.ts b/packages/core/data-transfer/src/engine/errors.ts index aedd1bfd47..799dbc63d4 100644 --- a/packages/core/data-transfer/src/engine/errors.ts +++ b/packages/core/data-transfer/src/engine/errors.ts @@ -30,4 +30,17 @@ class TransferEngineValidationError< } } -export { TransferEngineError, TransferEngineInitializationError, TransferEngineValidationError }; +class TransferEngineTransferError< + T extends { check: string } = { check: string } +> extends TransferEngineError<'transfer', T> { + constructor(message?: string, details?: T) { + super(SeverityKind.FATAL, message, { step: 'transfer', details }); + } +} + +export { + TransferEngineError, + TransferEngineInitializationError, + TransferEngineValidationError, + TransferEngineTransferError, +}; diff --git a/packages/core/data-transfer/src/engine/index.ts b/packages/core/data-transfer/src/engine/index.ts index 14f15e811d..68b0980bc3 100644 --- a/packages/core/data-transfer/src/engine/index.ts +++ b/packages/core/data-transfer/src/engine/index.ts @@ -30,6 +30,7 @@ import { IDiagnosticReporter, ErrorDiagnosticSeverity, } from './diagnostic'; +import { DataTransferError } from '../errors'; export const TRANSFER_STAGES: ReadonlyArray = Object.freeze([ 'entities', @@ -341,7 +342,9 @@ class TransferEngine< return `Modified "${diff.path}". "${diff.values[0]}" (${diff.types[0]}) => "${diff.values[1]}" (${diff.types[1]})`; } - throw new Error(`Invalid diff found for "${uid}"`); + throw new TransferEngineValidationError(`Invalid diff found for "${uid}"`, { + check: `schema on ${uid}`, + }); }) .map((line) => ` - ${line}`) .join(EOL); @@ -462,7 +465,7 @@ class TransferEngine< async close(): Promise { const results = await Promise.allSettled([ this.sourceProvider.close?.(), - this.destinationProvider.bootstrap?.(), + this.destinationProvider.close?.(), ]); results.forEach((result) => { @@ -541,13 +544,12 @@ class TransferEngine< this.#emitTransferUpdate('error', { error: e }); const lastDiagnostic = last(this.diagnostics.stack.items); - // Do not report an error diagnostic if the last one reported the same error if ( e instanceof Error && (!lastDiagnostic || lastDiagnostic.kind !== 'error' || lastDiagnostic.details.error !== e) ) { - this.#reportError(e, 'fatal'); + this.#reportError(e, (e as DataTransferError).severity || 'fatal'); } // Rollback the destination provider if an exception is thrown during the transfer diff --git a/packages/core/data-transfer/src/errors/constants.ts b/packages/core/data-transfer/src/errors/constants.ts index 6c3964cc3f..cae73c6e9a 100644 --- a/packages/core/data-transfer/src/errors/constants.ts +++ b/packages/core/data-transfer/src/errors/constants.ts @@ -1,6 +1,8 @@ -export const SeverityKind = { - FATAL: 1, - ERROR: 2, - SILLY: 3, +import { ErrorDiagnosticSeverity } from '../engine/diagnostic'; + +export const SeverityKind: Record = { + FATAL: 'fatal', + ERROR: 'error', + SILLY: 'silly', } as const; export type Severity = typeof SeverityKind[keyof typeof SeverityKind]; diff --git a/packages/core/data-transfer/src/errors/providers.ts b/packages/core/data-transfer/src/errors/providers.ts new file mode 100644 index 0000000000..d84b85083e --- /dev/null +++ b/packages/core/data-transfer/src/errors/providers.ts @@ -0,0 +1,40 @@ +import { DataTransferError } from './base'; +import { Severity, SeverityKind } from './constants'; + +type ProviderStep = 'initialization' | 'validation' | 'transfer'; + +type ProviderErrorDetails

= { + step: P; +} & ([U] extends [never] ? unknown : { details?: U }); + +export class ProviderError< + P extends ProviderStep = ProviderStep, + U = never, + T extends ProviderErrorDetails = ProviderErrorDetails +> extends DataTransferError { + constructor(severity: Severity, message?: string, details?: T | null) { + super('provider', severity, message, details); + } +} + +export class ProviderInitializationError extends ProviderError<'initialization'> { + constructor(message?: string) { + super(SeverityKind.FATAL, message, { step: 'initialization' }); + } +} + +// TODO: these types are not working correctly, ProviderTransferError() is accepting any details object rather than requiring T +export class ProviderValidationError extends ProviderError< + 'validation', + T +> { + constructor(message?: string, details?: T) { + super(SeverityKind.SILLY, message, { step: 'validation', details }); + } +} +// TODO: these types are not working correctly, ProviderTransferError() is accepting any details object rather than requiring T +export class ProviderTransferError extends ProviderError<'transfer', T> { + constructor(message?: string, details?: T) { + super(SeverityKind.FATAL, message, { step: 'transfer', details }); + } +} diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts index 9302354e41..eaa0f2e10f 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts @@ -5,6 +5,8 @@ import type { IAsset, IDestinationProvider, IMetadata, ProviderType } from '../. import { restore } from './strategies'; import * as utils from '../../../utils'; +import { ProviderTransferError, ProviderValidationError } from '../../../errors/providers'; +import { assertValidStrapi } from '../../../utils/providers'; export const VALID_CONFLICT_STRATEGIES = ['restore', 'merge']; export const DEFAULT_CONFLICT_STRATEGY = 'restore'; @@ -51,15 +53,16 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { #validateOptions() { if (!VALID_CONFLICT_STRATEGIES.includes(this.options.strategy)) { - throw new Error(`Invalid stategy ${this.options.strategy}`); + throw new ProviderValidationError(`Invalid stategy ${this.options.strategy}`, { + check: 'strategy', + strategy: this.options.strategy, + validStrategies: VALID_CONFLICT_STRATEGIES, + }); } } async #deleteAll() { - if (!this.strapi) { - throw new Error('Strapi instance not found'); - } - + assertValidStrapi(this.strapi); return restore.deleteRecords(this.strapi, this.options.restore); } @@ -89,10 +92,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } getSchemas() { - if (!this.strapi) { - throw new Error('Not able to get Schemas. Strapi instance not found'); - } - + assertValidStrapi(this.strapi, 'Not able to get Schemas.'); const schemas = { ...this.strapi.contentTypes, ...this.strapi.components, @@ -102,10 +102,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } createEntitiesWriteStream(): Writable { - if (!this.strapi) { - throw new Error('Not able to import entities. Strapi instance not found'); - } - + assertValidStrapi(this.strapi, 'Not able to import entities'); const { strategy } = this.options; const updateMappingTable = (type: string, oldID: number, newID: number) => { @@ -128,9 +125,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { // TODO: Move this logic to the restore strategy async createAssetsWriteStream(): Promise { - if (!this.strapi) { - throw new Error('Not able to stream Assets. Strapi instance not found'); - } + assertValidStrapi(this.strapi, 'Not able to stream Assets.'); const assetsDirectory = path.join(this.strapi.dirs.static.public, 'uploads'); const backupDirectory = path.join( @@ -159,12 +154,12 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { await fse.rm(assetsDirectory, { recursive: true, force: true }); await fse.rename(backupDirectory, assetsDirectory); this.destroy( - new Error( + new ProviderTransferError( `There was an error during the transfer process. The original files have been restored to ${assetsDirectory}` ) ); } catch (err) { - throw new Error( + throw new ProviderTransferError( `There was an error doing the rollback process. The original files are in ${backupDirectory}, but we failed to restore them to ${assetsDirectory}` ); } finally { @@ -176,17 +171,19 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } async createConfigurationWriteStream(): Promise { - if (!this.strapi) { - throw new Error('Not able to stream Configurations. Strapi instance not found'); - } + assertValidStrapi(this.strapi, 'Not able to stream Configurations.'); const { strategy } = this.options; - if (strategy === 'restore') { + if (strategy !== 'restore') { return restore.createConfigurationWriteStream(this.strapi); } - throw new Error(`Invalid strategy supplied: "${strategy}"`); + throw new ProviderValidationError(`Invalid stategy ${strategy}`, { + check: 'strategy', + strategy, + validStrategies: VALID_CONFLICT_STRATEGIES, + }); } async createLinksWriteStream(): Promise { @@ -201,7 +198,11 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { return restore.createLinksWriteStream(mapID, this.strapi); } - throw new Error(`Invalid strategy supplied: "${strategy}"`); + throw new ProviderValidationError(`Invalid stategy ${strategy}`, { + check: 'strategy', + strategy, + validStrategies: VALID_CONFLICT_STRATEGIES, + }); } } diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/configuration.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/configuration.ts index e1cb04b345..4e03c4fbfa 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/configuration.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/configuration.ts @@ -1,6 +1,7 @@ import { Writable } from 'stream'; import chalk from 'chalk'; import { IConfiguration } from '../../../../../../types'; +import { ProviderTransferError } from '../../../../../errors/providers'; const restoreCoreStore = async (strapi: Strapi.Strapi, data: T) => { return strapi.db.query('strapi::core-store').create({ @@ -37,7 +38,7 @@ export const createConfigurationWriteStream = async (strapi: Strapi.Strapi) => { await restoreConfigs(strapi, config); } catch (error) { return callback( - new Error( + new ProviderTransferError( `Failed to import ${chalk.yellowBright(config.type)} (${chalk.greenBright( config.value.id )}` diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/entities.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/entities.ts index c34403762e..49af337bc3 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/entities.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/entities.ts @@ -4,6 +4,7 @@ import { get, last } from 'lodash/fp'; import { Writable } from 'stream'; import type { IEntity } from '../../../../../../types'; +import { ProviderTransferError } from '../../../../../errors/providers'; import { json } from '../../../../../utils'; import * as queries from '../../../../queries'; @@ -90,7 +91,7 @@ const createEntitiesWriteStream = (options: IEntitiesRestoreStreamOptions) => { return callback(e); } - return callback(new Error(`Failed to create "${type}" (${id})`)); + return callback(new ProviderTransferError(`Failed to create "${type}" (${id})`)); } return callback(null); diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/index.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/index.ts index 90f9505fdb..3d4808a17c 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/index.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/index.ts @@ -1,4 +1,5 @@ import type { ContentTypeSchema } from '@strapi/strapi'; +import { ProviderTransferError } from '../../../../../errors/providers'; import * as queries from '../../../../queries'; export interface IRestoreOptions { @@ -116,7 +117,7 @@ const useResults = ( const update = (count: number, key?: string) => { if (key) { if (!(key in results.aggregate)) { - throw new Error(`Unknown key "${key}" provided in results update`); + throw new ProviderTransferError(`Unknown key "${key}" provided in results update`); } results.aggregate[key].count += count; diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/links.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/links.ts index 89e9b0b695..eafaa20c1a 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/links.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/strategies/restore/links.ts @@ -1,5 +1,6 @@ import { Writable } from 'stream'; import { ILink } from '../../../../../../types'; +import { ProviderTransferError } from '../../../../../errors/providers'; import { createLinkQuery } from '../../../../queries/link'; export const createLinksWriteStream = ( @@ -24,7 +25,7 @@ export const createLinksWriteStream = ( } return callback( - new Error(`An error happened while trying to import a ${left.type} link. ${e}`) + new ProviderTransferError(`An error happened while trying to import a ${left.type} link.`) ); } diff --git a/packages/core/data-transfer/src/strapi/providers/local-source/index.ts b/packages/core/data-transfer/src/strapi/providers/local-source/index.ts index cc3e07dd8f..b4e928eaa8 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-source/index.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-source/index.ts @@ -7,6 +7,7 @@ import { createLinksStream } from './links'; import { createConfigurationStream } from './configuration'; import { createAssetsStream } from './assets'; import * as utils from '../../../utils'; +import { assertValidStrapi } from '../../../utils/providers'; export interface ILocalStrapiSourceProviderOptions { getStrapi(): Strapi.Strapi | Promise; @@ -64,9 +65,7 @@ class LocalStrapiSourceProvider implements ISourceProvider { } async createEntitiesReadStream(): Promise { - if (!this.strapi) { - throw new Error('Not able to stream entities. Strapi instance not found'); - } + assertValidStrapi(this.strapi, 'Not able to stream entities.'); return chain([ // Entities stream @@ -78,25 +77,19 @@ class LocalStrapiSourceProvider implements ISourceProvider { } createLinksReadStream(): Readable { - if (!this.strapi) { - throw new Error('Not able to stream links. Strapi instance not found'); - } + assertValidStrapi(this.strapi, 'Not able to stream links.'); return createLinksStream(this.strapi); } createConfigurationReadStream(): Readable { - if (!this.strapi) { - throw new Error('Not able to stream configuration. Strapi instance not found'); - } + assertValidStrapi(this.strapi, 'Not able to stream configuration. Strapi instance not found'); return createConfigurationStream(strapi); } getSchemas() { - if (!this.strapi) { - throw new Error('Not able to get Schemas. Strapi instance not found'); - } + assertValidStrapi(this.strapi, 'Not able to get Schemas'); const schemas = { ...this.strapi.contentTypes, @@ -111,9 +104,7 @@ class LocalStrapiSourceProvider implements ISourceProvider { } createAssetsReadStream(): Readable { - if (!this.strapi) { - throw new Error('Not able to stream assets. Strapi instance not found'); - } + assertValidStrapi(this.strapi, 'Not able to stream assets'); return createAssetsStream(this.strapi); } diff --git a/packages/core/data-transfer/src/strapi/providers/remote-destination/index.ts b/packages/core/data-transfer/src/strapi/providers/remote-destination/index.ts index a83b27c6be..8987a650e6 100644 --- a/packages/core/data-transfer/src/strapi/providers/remote-destination/index.ts +++ b/packages/core/data-transfer/src/strapi/providers/remote-destination/index.ts @@ -16,6 +16,7 @@ import type { import type { client, server } from '../../../../types/remote/protocol'; import type { ILocalStrapiDestinationProviderOptions } from '../local-destination'; import { TRANSFER_PATH } from '../../remote/constants'; +import { ProviderTransferError, ProviderValidationError } from '../../../errors/providers'; interface ITokenAuth { type: 'token'; @@ -66,7 +67,9 @@ class RemoteStrapiDestinationProvider implements IDestinationProvider { const res = (await query) as server.Payload; if (!res?.transferID) { - return reject(new Error('Init failed, invalid response from the server')); + return reject( + new ProviderTransferError('Init failed, invalid response from the server') + ); } resolve(res.transferID); @@ -87,10 +90,10 @@ class RemoteStrapiDestinationProvider implements IDestinationProvider { } if (typeof e === 'string') { - return new Error(e); + return new ProviderTransferError(e); } - return new Error('Unexpected error'); + return new ProviderTransferError('Unexpected error'); } return null; @@ -98,14 +101,22 @@ class RemoteStrapiDestinationProvider implements IDestinationProvider { async bootstrap(): Promise { const { url, auth } = this.options; + const validProtocols = ['https:', 'http:']; let ws: WebSocket; - if (!['https:', 'http:'].includes(url.protocol)) { - throw new Error(`Invalid protocol "${url.protocol}"`); + if (!validProtocols.includes(url.protocol)) { + throw new ProviderValidationError(`Invalid protocol "${url.protocol}"`, { + check: 'url', + details: { + protocol: url.protocol, + validProtocols, + }, + }); } const wsProtocol = url.protocol === 'https:' ? 'wss:' : 'ws:'; const wsUrl = `${wsProtocol}//${url.host}${url.pathname}${TRANSFER_PATH}`; + const validAuthMethods = ['token']; // No auth defined, trying public access for transfer if (!auth) { @@ -120,7 +131,13 @@ class RemoteStrapiDestinationProvider implements IDestinationProvider { // Invalid auth method provided else { - throw new Error('Auth method not implemented'); + throw new ProviderValidationError('Auth method not implemented', { + check: 'auth.type', + details: { + auth: auth.type, + validAuthMethods, + }, + }); } this.ws = ws; diff --git a/packages/core/data-transfer/src/strapi/remote/constants.ts b/packages/core/data-transfer/src/strapi/remote/constants.ts index 536be055a2..4b8400ece4 100644 --- a/packages/core/data-transfer/src/strapi/remote/constants.ts +++ b/packages/core/data-transfer/src/strapi/remote/constants.ts @@ -1 +1,2 @@ export const TRANSFER_PATH = '/transfer'; +export const TRANSFER_METHODS = ['push', 'pull']; diff --git a/packages/core/data-transfer/src/strapi/remote/handlers.ts b/packages/core/data-transfer/src/strapi/remote/handlers.ts index f6c9315b21..f0d91dc144 100644 --- a/packages/core/data-transfer/src/strapi/remote/handlers.ts +++ b/packages/core/data-transfer/src/strapi/remote/handlers.ts @@ -8,6 +8,8 @@ import type { IPushController } from './controllers/push'; import createPushController from './controllers/push'; import type { client, server } from '../../../types/remote/protocol'; +import { ProviderTransferError, ProviderInitializationError } from '../../errors/providers'; +import { TRANSFER_METHODS } from './constants'; interface ITransferState { transfer?: { id: string; kind: client.TransferKind }; @@ -68,9 +70,13 @@ export const createTransferHandler = if (e instanceof Error) { callback(e); } else if (typeof e === 'string') { - callback(new Error(e)); + callback(new ProviderTransferError(e)); } else { - callback(new Error('Unexpected error')); + callback( + new ProviderTransferError('Unexpected error', { + error: e, + }) + ); } } }; @@ -83,8 +89,9 @@ export const createTransferHandler = }; const init = (msg: client.InitCommand): server.Payload => { + // TODO: this only checks for this instance of node: we should consider a database lock if (state.controller) { - throw new Error('Transfer already in progres'); + throw new ProviderInitializationError('Transfer already in progres'); } const { transfer } = msg.params; @@ -102,7 +109,10 @@ export const createTransferHandler = // Pull or any other string else { - throw new Error(`Transfer not implemented: "${transfer}"`); + throw new ProviderTransferError(`Transfer type not implemented: "${transfer}"`, { + transfer, + validTransfers: TRANSFER_METHODS, + }); } state.transfer = { id: randomUUID(), kind: transfer }; @@ -125,7 +135,12 @@ export const createTransferHandler = } if (command === 'status') { - await callback(new Error('Command not implemented: "status"')); + await callback( + new ProviderTransferError('Command not implemented: "status"', { + command, + validCommands: ['init', 'end', 'status'], + }) + ); } }; @@ -137,15 +152,15 @@ export const createTransferHandler = // It shouldn't be possible to strart a pull transfer for now, so reaching // this code should be impossible too, but this has been added by security if (state.transfer?.kind === 'pull') { - return callback(new Error('Pull transfer not implemented')); + return callback(new ProviderTransferError('Pull transfer not implemented')); } if (!controller) { - return callback(new Error("The transfer hasn't been initialized")); + return callback(new ProviderTransferError("The transfer hasn't been initialized")); } if (!transferID) { - return callback(new Error('Missing transfer ID')); + return callback(new ProviderTransferError('Missing transfer ID')); } // Action @@ -153,7 +168,12 @@ export const createTransferHandler = const { action } = msg; if (!(action in controller.actions)) { - return callback(new Error(`Invalid action provided: "${action}"`)); + return callback( + new ProviderTransferError(`Invalid action provided: "${action}"`, { + action, + validActions: Object.keys(controller.actions), + }) + ); } await answer(() => controller.actions[action as keyof typeof controller.actions]()); @@ -187,6 +207,7 @@ export const createTransferHandler = ws.on('error', (e) => { teardown(); + // TODO: is logging a console error to the running instance of Strapi ok to do? Should we check for an existing strapi.logger to use? console.error(e); }); @@ -194,7 +215,8 @@ export const createTransferHandler = const msg: client.Message = JSON.parse(raw.toString()); if (!msg.uuid) { - throw new Error('Missing uuid in message'); + await callback(new ProviderTransferError('Missing uuid in message')); + return; } uuid = msg.uuid; @@ -211,7 +233,7 @@ export const createTransferHandler = // Invalid messages else { - await callback(new Error('Bad request')); + await callback(new ProviderTransferError('Bad request')); } }); }); diff --git a/packages/core/data-transfer/src/utils/providers.ts b/packages/core/data-transfer/src/utils/providers.ts new file mode 100644 index 0000000000..a93d84a8d5 --- /dev/null +++ b/packages/core/data-transfer/src/utils/providers.ts @@ -0,0 +1,15 @@ +import { ProviderInitializationError } from '../errors/providers'; + +export type ValidStrapiAssertion = ( + strapi: unknown, + msg?: string +) => asserts strapi is Strapi.Strapi; + +export const assertValidStrapi: ValidStrapiAssertion = ( + strapi?: unknown, + msg = '' +): asserts strapi => { + if (!strapi) { + throw new ProviderInitializationError(`Strapi instance not found. ${msg}`); + } +}; diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index fc5029d0e9..33757af449 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -11,12 +11,14 @@ const { isObject, isString, isFinite, toNumber } = require('lodash/fp'); const fs = require('fs-extra'); const chalk = require('chalk'); +const { TransferEngineTransferError } = require('@strapi/data-transfer/lib/engine/errors'); const { getDefaultExportName, buildTransferTable, DEFAULT_IGNORED_CONTENT_TYPES, createStrapiInstance, } = require('./utils'); +const formatDiagnosticErrors = require('../utils/formatter'); /** * @typedef ImportCommandOptions Options given to the CLI import command @@ -74,28 +76,7 @@ module.exports = async (opts) => { }, }); - engine.diagnostics - .on('error', ({ details }) => { - const { createdAt, severity, name, message } = details; - - logger.error( - chalk.red(`[${createdAt.toISOString()}] [error] (${severity}) ${name}: ${message}`) - ); - }) - .on('info', ({ details }) => { - const { createdAt, message, params } = details; - - const msg = typeof message === 'function' ? message(params) : message; - - logger.info(chalk.blue(`[${createdAt.toISOString()}] [info] ${msg}`)); - }) - .on('warning', ({ details }) => { - const { createdAt, origin, message } = details; - - logger.warn( - chalk.yellow(`[${createdAt.toISOString()}] [warning] (${origin ?? 'transfer'}) ${message}`) - ); - }); + engine.diagnostics.onDiagnostic(formatDiagnosticErrors); const progress = engine.progress.stream; @@ -122,7 +103,7 @@ module.exports = async (opts) => { const outFileExists = await fs.pathExists(outFile); if (!outFileExists) { - throw new Error(`Export file not created "${outFile}"`); + throw new TransferEngineTransferError(`Export file not created "${outFile}"`); } logger.log(`${chalk.bold('Export process has been completed successfully!')}`); diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index 61e36b228b..7f1f7296e7 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -14,10 +14,10 @@ const { const { isObject } = require('lodash/fp'); const path = require('path'); -const chalk = require('chalk'); const strapi = require('../../index'); const { buildTransferTable, DEFAULT_IGNORED_CONTENT_TYPES } = require('./utils'); +const formatDiagnosticErrors = require('../utils/formatter'); /** * @typedef {import('@strapi/data-transfer').ILocalFileSourceProviderOptions} ILocalFileSourceProviderOptions @@ -84,28 +84,7 @@ module.exports = async (opts) => { const engine = createTransferEngine(source, destination, engineOptions); - engine.diagnostics - .on('error', ({ details }) => { - const { createdAt, severity, name, message } = details; - - logger.error( - chalk.red(`[${createdAt.toISOString()}] [error] (${severity}) ${name}: ${message}`) - ); - }) - .on('info', ({ details }) => { - const { createdAt, message, params } = details; - - const msg = typeof message === 'function' ? message(params) : message; - - logger.info(chalk.blue(`[${createdAt.toISOString()}] [info] ${msg}`)); - }) - .on('warning', ({ details }) => { - const { createdAt, origin, message } = details; - - logger.warn( - chalk.yellow(`[${createdAt.toISOString()}] [warning] (${origin ?? 'transfer'}) ${message}`) - ); - }); + engine.diagnostics.onDiagnostic(formatDiagnosticErrors); const progress = engine.progress.stream; const getTelemetryPayload = () => { diff --git a/packages/core/strapi/lib/commands/transfer/transfer.js b/packages/core/strapi/lib/commands/transfer/transfer.js index d97a6b9829..812b69600f 100644 --- a/packages/core/strapi/lib/commands/transfer/transfer.js +++ b/packages/core/strapi/lib/commands/transfer/transfer.js @@ -11,13 +11,15 @@ const { const { isObject } = require('lodash/fp'); const chalk = require('chalk'); +const { createLogger } = require('@strapi/logger'); const { buildTransferTable, createStrapiInstance, DEFAULT_IGNORED_CONTENT_TYPES, } = require('./utils'); +const formatDiagnosticErrors = require('../utils/formatter'); -const logger = console; +const logger = createLogger(); /** * @typedef TransferCommandOptions Options given to the CLI transfer command @@ -109,6 +111,8 @@ module.exports = async (opts) => { }, }); + engine.diagnostics.onDiagnostic(formatDiagnosticErrors); + try { logger.log(`Starting transfer...`); diff --git a/packages/core/strapi/lib/commands/utils/formatter.js b/packages/core/strapi/lib/commands/utils/formatter.js new file mode 100644 index 0000000000..084d25d2b8 --- /dev/null +++ b/packages/core/strapi/lib/commands/utils/formatter.js @@ -0,0 +1,55 @@ +'use strict'; + +const { createLogger, createOutputFileConfiguration } = require('@strapi/logger'); +const chalk = require('chalk'); + +const logger = createLogger(createOutputFileConfiguration('data-transfer.log')); + +const errorColors = { + fatal: chalk.red, + error: chalk.red, + silly: chalk.yellow, +}; + +const formatDiagnosticErrors = ({ details, kind }) => { + try { + if (kind === 'error') { + const { message, severity = 'fatal', error, details: moreDetails } = details; + + const detailsInfo = error ?? moreDetails; + let errorMessage = errorColors[severity](`[${severity.toUpperCase()}] ${message}`); + if (detailsInfo && detailsInfo.details) { + const { + origin, + details: { step, details: stepDetails, ...moreInfo }, + } = detailsInfo; + errorMessage = `${errorMessage}. Thrown at ${origin} during ${step}.\n`; + if (stepDetails || moreInfo) { + const { check, ...info } = stepDetails ?? moreInfo; + errorMessage = `${errorMessage} Check ${check ?? ''}: ${JSON.stringify(info, null, 2)}`; + } + } + + logger.error(new Error(errorMessage, error)); + } + if (kind === 'info') { + const { message, params } = details; + + const msg = + typeof message === 'function' + ? message(params) + : `${message}\n${params ? JSON.stringify(params, null, 2) : ''}`; + + logger.info(msg); + } + if (kind === 'warning') { + const { origin, message } = details; + + logger.warn(`(${origin ?? 'transfer'}) ${message}`); + } + } catch (err) { + logger.error(err); + } +}; + +module.exports = formatDiagnosticErrors; diff --git a/packages/utils/logger/lib/index.js b/packages/utils/logger/lib/index.js index fec80cf5ba..419b79fdc2 100644 --- a/packages/utils/logger/lib/index.js +++ b/packages/utils/logger/lib/index.js @@ -4,6 +4,7 @@ const winston = require('winston'); const formats = require('./formats'); const createDefaultConfiguration = require('./default-configuration'); +const createOutputFileConfiguration = require('./output-file-configuration'); const createLogger = (userConfiguration = {}) => { const configuration = createDefaultConfiguration(); @@ -13,4 +14,4 @@ const createLogger = (userConfiguration = {}) => { return winston.createLogger(configuration); }; -module.exports = { createLogger, winston, formats }; +module.exports = { createLogger, winston, formats, createOutputFileConfiguration }; diff --git a/packages/utils/logger/lib/output-file-configuration.js b/packages/utils/logger/lib/output-file-configuration.js new file mode 100644 index 0000000000..628ec32880 --- /dev/null +++ b/packages/utils/logger/lib/output-file-configuration.js @@ -0,0 +1,24 @@ +'use strict'; + +const { transports, format } = require('winston'); +const { LEVEL_LABEL, LEVELS } = require('./constants'); +const { prettyPrint } = require('./formats'); + +const getPlainText = format.printf(({ message }) => { + return message.replace( + // eslint-disable-next-line no-control-regex + /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g, + '' + ); +}); + +const createOutputFileConfiguration = (filename) => { + return { + level: LEVEL_LABEL, + levels: LEVELS, + format: prettyPrint(), + transports: [new transports.Console(), new transports.File({ filename, format: getPlainText })], + }; +}; + +module.exports = createOutputFileConfiguration; From a55ffde7b8762bd8703b7657c18ecd6e9c0b8274 Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Mon, 23 Jan 2023 17:25:22 +0100 Subject: [PATCH 08/16] Move deits formatter to the deits folder --- .../providers/local-destination/index.ts | 14 +++-- .../strapi/lib/commands/transfer/export.js | 2 +- .../strapi/lib/commands/transfer/import.js | 7 ++- .../strapi/lib/commands/transfer/transfer.js | 2 +- .../strapi/lib/commands/transfer/utils.js | 51 +++++++++++++++++ .../strapi/lib/commands/utils/formatter.js | 55 ------------------- .../logger/lib/output-file-configuration.js | 5 +- 7 files changed, 71 insertions(+), 65 deletions(-) delete mode 100644 packages/core/strapi/lib/commands/utils/formatter.js diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts index eaa0f2e10f..a8bd6eee24 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts @@ -53,7 +53,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { #validateOptions() { if (!VALID_CONFLICT_STRATEGIES.includes(this.options.strategy)) { - throw new ProviderValidationError(`Invalid stategy ${this.options.strategy}`, { + throw new ProviderValidationError(`Invalid strategy ${this.options.strategy}`, { check: 'strategy', strategy: this.options.strategy, validStrategies: VALID_CONFLICT_STRATEGIES, @@ -120,7 +120,11 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { }); } - throw new Error(`Invalid strategy supplied: "${strategy}"`); + throw new ProviderValidationError(`Invalid strategy ${this.options.strategy}`, { + check: 'strategy', + strategy: this.options.strategy, + validStrategies: VALID_CONFLICT_STRATEGIES, + }); } // TODO: Move this logic to the restore strategy @@ -175,11 +179,11 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { const { strategy } = this.options; - if (strategy !== 'restore') { + if (strategy === 'restore') { return restore.createConfigurationWriteStream(this.strapi); } - throw new ProviderValidationError(`Invalid stategy ${strategy}`, { + throw new ProviderValidationError(`Invalid strategy ${strategy}`, { check: 'strategy', strategy, validStrategies: VALID_CONFLICT_STRATEGIES, @@ -198,7 +202,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { return restore.createLinksWriteStream(mapID, this.strapi); } - throw new ProviderValidationError(`Invalid stategy ${strategy}`, { + throw new ProviderValidationError(`Invalid strategy ${strategy}`, { check: 'strategy', strategy, validStrategies: VALID_CONFLICT_STRATEGIES, diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 0a9c0f319e..8f3d70cd8f 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -17,8 +17,8 @@ const { buildTransferTable, DEFAULT_IGNORED_CONTENT_TYPES, createStrapiInstance, + formatDiagnosticErrors, } = require('./utils'); -const formatDiagnosticErrors = require('../utils/formatter'); /** * @typedef ImportCommandOptions Options given to the CLI import command diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index 32545fdbad..74605a710f 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -16,8 +16,11 @@ const { isObject } = require('lodash/fp'); const path = require('path'); const strapi = require('../../index'); -const { buildTransferTable, DEFAULT_IGNORED_CONTENT_TYPES } = require('./utils'); -const formatDiagnosticErrors = require('../utils/formatter'); +const { + buildTransferTable, + DEFAULT_IGNORED_CONTENT_TYPES, + formatDiagnosticErrors, +} = require('./utils'); /** * @typedef {import('@strapi/data-transfer').ILocalFileSourceProviderOptions} ILocalFileSourceProviderOptions diff --git a/packages/core/strapi/lib/commands/transfer/transfer.js b/packages/core/strapi/lib/commands/transfer/transfer.js index 812b69600f..632ea06a13 100644 --- a/packages/core/strapi/lib/commands/transfer/transfer.js +++ b/packages/core/strapi/lib/commands/transfer/transfer.js @@ -17,7 +17,7 @@ const { createStrapiInstance, DEFAULT_IGNORED_CONTENT_TYPES, } = require('./utils'); -const formatDiagnosticErrors = require('../utils/formatter'); +const formatDiagnosticErrors = require('./utils'); const logger = createLogger(); diff --git a/packages/core/strapi/lib/commands/transfer/utils.js b/packages/core/strapi/lib/commands/transfer/utils.js index d19d66a0af..393e2bbd92 100644 --- a/packages/core/strapi/lib/commands/transfer/utils.js +++ b/packages/core/strapi/lib/commands/transfer/utils.js @@ -4,6 +4,7 @@ const chalk = require('chalk'); const Table = require('cli-table3'); const { Option } = require('commander'); const { TransferGroupPresets } = require('@strapi/data-transfer/lib/engine'); +const { createLogger, createOutputFileConfiguration } = require('@strapi/logger'); const { readableBytes, exitWith } = require('../utils/helpers'); const strapi = require('../../index'); const { getParseListWithChoices } = require('../utils/commander'); @@ -121,6 +122,55 @@ const validateExcludeOnly = (command) => { } }; +const logger = createLogger(createOutputFileConfiguration(`import_error_log_${Date.now()}.log`)); + +const errorColors = { + fatal: chalk.red, + error: chalk.red, + silly: chalk.yellow, +}; + +const formatDiagnosticErrors = ({ details, kind }) => { + try { + if (kind === 'error') { + const { message, severity = 'fatal', error, details: moreDetails } = details; + + const detailsInfo = error ?? moreDetails; + let errorMessage = errorColors[severity](`[${severity.toUpperCase()}] ${message}`); + if (detailsInfo && detailsInfo.details) { + const { + origin, + details: { step, details: stepDetails, ...moreInfo }, + } = detailsInfo; + errorMessage = `${errorMessage}. Thrown at ${origin} during ${step}.\n`; + if (stepDetails || moreInfo) { + const { check, ...info } = stepDetails ?? moreInfo; + errorMessage = `${errorMessage} Check ${check ?? ''}: ${JSON.stringify(info, null, 2)}`; + } + } + + logger.error(new Error(errorMessage, error)); + } + if (kind === 'info') { + const { message, params } = details; + + const msg = + typeof message === 'function' + ? message(params) + : `${message}\n${params ? JSON.stringify(params, null, 2) : ''}`; + + logger.info(msg); + } + if (kind === 'warning') { + const { origin, message } = details; + + logger.warn(`(${origin ?? 'transfer'}) ${message}`); + } + } catch (err) { + logger.error(err); + } +}; + module.exports = { buildTransferTable, getDefaultExportName, @@ -129,4 +179,5 @@ module.exports = { excludeOption, onlyOption, validateExcludeOnly, + formatDiagnosticErrors, }; diff --git a/packages/core/strapi/lib/commands/utils/formatter.js b/packages/core/strapi/lib/commands/utils/formatter.js deleted file mode 100644 index 084d25d2b8..0000000000 --- a/packages/core/strapi/lib/commands/utils/formatter.js +++ /dev/null @@ -1,55 +0,0 @@ -'use strict'; - -const { createLogger, createOutputFileConfiguration } = require('@strapi/logger'); -const chalk = require('chalk'); - -const logger = createLogger(createOutputFileConfiguration('data-transfer.log')); - -const errorColors = { - fatal: chalk.red, - error: chalk.red, - silly: chalk.yellow, -}; - -const formatDiagnosticErrors = ({ details, kind }) => { - try { - if (kind === 'error') { - const { message, severity = 'fatal', error, details: moreDetails } = details; - - const detailsInfo = error ?? moreDetails; - let errorMessage = errorColors[severity](`[${severity.toUpperCase()}] ${message}`); - if (detailsInfo && detailsInfo.details) { - const { - origin, - details: { step, details: stepDetails, ...moreInfo }, - } = detailsInfo; - errorMessage = `${errorMessage}. Thrown at ${origin} during ${step}.\n`; - if (stepDetails || moreInfo) { - const { check, ...info } = stepDetails ?? moreInfo; - errorMessage = `${errorMessage} Check ${check ?? ''}: ${JSON.stringify(info, null, 2)}`; - } - } - - logger.error(new Error(errorMessage, error)); - } - if (kind === 'info') { - const { message, params } = details; - - const msg = - typeof message === 'function' - ? message(params) - : `${message}\n${params ? JSON.stringify(params, null, 2) : ''}`; - - logger.info(msg); - } - if (kind === 'warning') { - const { origin, message } = details; - - logger.warn(`(${origin ?? 'transfer'}) ${message}`); - } - } catch (err) { - logger.error(err); - } -}; - -module.exports = formatDiagnosticErrors; diff --git a/packages/utils/logger/lib/output-file-configuration.js b/packages/utils/logger/lib/output-file-configuration.js index 628ec32880..ce4e97f084 100644 --- a/packages/utils/logger/lib/output-file-configuration.js +++ b/packages/utils/logger/lib/output-file-configuration.js @@ -17,7 +17,10 @@ const createOutputFileConfiguration = (filename) => { level: LEVEL_LABEL, levels: LEVELS, format: prettyPrint(), - transports: [new transports.Console(), new transports.File({ filename, format: getPlainText })], + transports: [ + new transports.Console(), + new transports.File({ level: 'error', filename, format: getPlainText }), + ], }; }; From 16d62df76abc86429780dbfd0fdf24867b10f419 Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Mon, 23 Jan 2023 17:39:40 +0100 Subject: [PATCH 09/16] Fix small issues --- .../core/data-transfer/src/engine/diagnostic.ts | 5 +---- .../core/data-transfer/types/transfer-engine.d.ts | 3 +-- .../core/strapi/lib/commands/transfer/transfer.js | 3 +-- .../core/strapi/lib/commands/transfer/utils.js | 14 +++++++------- .../lib/{ => config}/default-configuration.js | 4 ++-- .../lib/{ => config}/output-file-configuration.js | 8 ++++++-- packages/utils/logger/lib/index.js | 11 ++++++++--- 7 files changed, 26 insertions(+), 22 deletions(-) rename packages/utils/logger/lib/{ => config}/default-configuration.js (73%) rename packages/utils/logger/lib/{ => config}/output-file-configuration.js (73%) diff --git a/packages/core/data-transfer/src/engine/diagnostic.ts b/packages/core/data-transfer/src/engine/diagnostic.ts index d4dc193fac..7f303d9ab2 100644 --- a/packages/core/data-transfer/src/engine/diagnostic.ts +++ b/packages/core/data-transfer/src/engine/diagnostic.ts @@ -79,10 +79,7 @@ const createDiagnosticReporter = ( }; const isDiagnosticValid = (diagnostic: Diagnostic) => { - if (!diagnostic.kind || !diagnostic.details) { - return false; - } - if (!diagnostic.details.message) { + if (!diagnostic.kind || !diagnostic.details || !diagnostic.details.message) { return false; } return true; diff --git a/packages/core/data-transfer/types/transfer-engine.d.ts b/packages/core/data-transfer/types/transfer-engine.d.ts index 10e8b33e0e..9ffea8ff4a 100644 --- a/packages/core/data-transfer/types/transfer-engine.d.ts +++ b/packages/core/data-transfer/types/transfer-engine.d.ts @@ -1,9 +1,8 @@ -import { PassThrough } from 'stream'; +import type { PassThrough } from 'stream'; import type { IAsset, IEntity, ILink } from './common-entities'; import type { ITransferResults, TransferTransform, TransferTransforms } from './utils'; import type { ISourceProvider, IDestinationProvider } from './providers'; import type { Schema } from '@strapi/strapi'; -import type { IAsset, IEntity, ILink } from './common-entities'; import type { ITransferResults, TransferTransform, TransferProgress } from './utils'; import type { ISourceProvider, IDestinationProvider } from './providers'; import type { Severity } from '../src/errors'; diff --git a/packages/core/strapi/lib/commands/transfer/transfer.js b/packages/core/strapi/lib/commands/transfer/transfer.js index 632ea06a13..4c0743f8d4 100644 --- a/packages/core/strapi/lib/commands/transfer/transfer.js +++ b/packages/core/strapi/lib/commands/transfer/transfer.js @@ -11,7 +11,6 @@ const { const { isObject } = require('lodash/fp'); const chalk = require('chalk'); -const { createLogger } = require('@strapi/logger'); const { buildTransferTable, createStrapiInstance, @@ -19,7 +18,7 @@ const { } = require('./utils'); const formatDiagnosticErrors = require('./utils'); -const logger = createLogger(); +const logger = console; /** * @typedef TransferCommandOptions Options given to the CLI transfer command diff --git a/packages/core/strapi/lib/commands/transfer/utils.js b/packages/core/strapi/lib/commands/transfer/utils.js index 393e2bbd92..083d371953 100644 --- a/packages/core/strapi/lib/commands/transfer/utils.js +++ b/packages/core/strapi/lib/commands/transfer/utils.js @@ -4,7 +4,10 @@ const chalk = require('chalk'); const Table = require('cli-table3'); const { Option } = require('commander'); const { TransferGroupPresets } = require('@strapi/data-transfer/lib/engine'); -const { createLogger, createOutputFileConfiguration } = require('@strapi/logger'); +const { + config: { createOutputFileConfiguration }, + createLogger, +} = require('@strapi/logger'); const { readableBytes, exitWith } = require('../utils/helpers'); const strapi = require('../../index'); const { getParseListWithChoices } = require('../utils/commander'); @@ -130,7 +133,7 @@ const errorColors = { silly: chalk.yellow, }; -const formatDiagnosticErrors = ({ details, kind }) => { +const formatDiagnostic = ({ details, kind }) => { try { if (kind === 'error') { const { message, severity = 'fatal', error, details: moreDetails } = details; @@ -154,10 +157,7 @@ const formatDiagnosticErrors = ({ details, kind }) => { if (kind === 'info') { const { message, params } = details; - const msg = - typeof message === 'function' - ? message(params) - : `${message}\n${params ? JSON.stringify(params, null, 2) : ''}`; + const msg = `${message}\n${params ? JSON.stringify(params, null, 2) : ''}`; logger.info(msg); } @@ -179,5 +179,5 @@ module.exports = { excludeOption, onlyOption, validateExcludeOnly, - formatDiagnosticErrors, + formatDiagnosticErrors: formatDiagnostic, }; diff --git a/packages/utils/logger/lib/default-configuration.js b/packages/utils/logger/lib/config/default-configuration.js similarity index 73% rename from packages/utils/logger/lib/default-configuration.js rename to packages/utils/logger/lib/config/default-configuration.js index 39c7f91047..b2005d24f4 100644 --- a/packages/utils/logger/lib/default-configuration.js +++ b/packages/utils/logger/lib/config/default-configuration.js @@ -1,8 +1,8 @@ 'use strict'; const { transports } = require('winston'); -const { LEVEL_LABEL, LEVELS } = require('./constants'); -const { prettyPrint } = require('./formats'); +const { LEVEL_LABEL, LEVELS } = require('../constants'); +const { prettyPrint } = require('../formats'); const createDefaultConfiguration = () => { return { diff --git a/packages/utils/logger/lib/output-file-configuration.js b/packages/utils/logger/lib/config/output-file-configuration.js similarity index 73% rename from packages/utils/logger/lib/output-file-configuration.js rename to packages/utils/logger/lib/config/output-file-configuration.js index ce4e97f084..87f34ea026 100644 --- a/packages/utils/logger/lib/output-file-configuration.js +++ b/packages/utils/logger/lib/config/output-file-configuration.js @@ -1,9 +1,13 @@ 'use strict'; const { transports, format } = require('winston'); -const { LEVEL_LABEL, LEVELS } = require('./constants'); -const { prettyPrint } = require('./formats'); +const { LEVEL_LABEL, LEVELS } = require('../constants'); +const { prettyPrint } = require('../formats'); +/** + * This will remove the chalk color codes from the message provided. + * It's used to log plain text in the log file + */ const getPlainText = format.printf(({ message }) => { return message.replace( // eslint-disable-next-line no-control-regex diff --git a/packages/utils/logger/lib/index.js b/packages/utils/logger/lib/index.js index 419b79fdc2..1077dbeb14 100644 --- a/packages/utils/logger/lib/index.js +++ b/packages/utils/logger/lib/index.js @@ -3,8 +3,8 @@ const winston = require('winston'); const formats = require('./formats'); -const createDefaultConfiguration = require('./default-configuration'); -const createOutputFileConfiguration = require('./output-file-configuration'); +const createDefaultConfiguration = require('./config/default-configuration'); +const createOutputFileConfiguration = require('./config/output-file-configuration'); const createLogger = (userConfiguration = {}) => { const configuration = createDefaultConfiguration(); @@ -14,4 +14,9 @@ const createLogger = (userConfiguration = {}) => { return winston.createLogger(configuration); }; -module.exports = { createLogger, winston, formats, createOutputFileConfiguration }; +module.exports = { + createLogger, + winston, + formats, + config: { createDefaultConfiguration, createOutputFileConfiguration }, +}; From 39c8a80339f2e4eb6955fd37fe5aa6e2388bb0be Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Tue, 24 Jan 2023 10:59:45 +0100 Subject: [PATCH 10/16] Fix file structure --- .../strapi/lib/commands/transfer/export.js | 4 +-- .../strapi/lib/commands/transfer/import.js | 8 ++--- .../strapi/lib/commands/transfer/transfer.js | 4 +-- .../strapi/lib/commands/transfer/utils.js | 7 +++-- .../lib/config/output-file-configuration.js | 31 ------------------- .../default-configuration.js | 0 packages/utils/logger/lib/configs/index.js | 9 ++++++ .../lib/configs/output-file-configuration.js | 20 ++++++++++++ .../logger/lib/formats/exclude-colors.js | 17 ++++++++++ packages/utils/logger/lib/index.js | 7 ++--- 10 files changed, 59 insertions(+), 48 deletions(-) delete mode 100644 packages/utils/logger/lib/config/output-file-configuration.js rename packages/utils/logger/lib/{config => configs}/default-configuration.js (100%) create mode 100644 packages/utils/logger/lib/configs/index.js create mode 100644 packages/utils/logger/lib/configs/output-file-configuration.js create mode 100644 packages/utils/logger/lib/formats/exclude-colors.js diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 8f3d70cd8f..9bdf88773d 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -17,7 +17,7 @@ const { buildTransferTable, DEFAULT_IGNORED_CONTENT_TYPES, createStrapiInstance, - formatDiagnosticErrors, + formatDiagnostic, } = require('./utils'); /** @@ -78,7 +78,7 @@ module.exports = async (opts) => { }, }); - engine.diagnostics.onDiagnostic(formatDiagnosticErrors); + engine.diagnostics.onDiagnostic(formatDiagnostic); const progress = engine.progress.stream; diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index 74605a710f..f1d05078df 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -16,11 +16,7 @@ const { isObject } = require('lodash/fp'); const path = require('path'); const strapi = require('../../index'); -const { - buildTransferTable, - DEFAULT_IGNORED_CONTENT_TYPES, - formatDiagnosticErrors, -} = require('./utils'); +const { buildTransferTable, DEFAULT_IGNORED_CONTENT_TYPES, formatDiagnostic } = require('./utils'); /** * @typedef {import('@strapi/data-transfer').ILocalFileSourceProviderOptions} ILocalFileSourceProviderOptions @@ -88,7 +84,7 @@ module.exports = async (opts) => { const engine = createTransferEngine(source, destination, engineOptions); - engine.diagnostics.onDiagnostic(formatDiagnosticErrors); + engine.diagnostics.onDiagnostic(formatDiagnostic); const progress = engine.progress.stream; const getTelemetryPayload = () => { diff --git a/packages/core/strapi/lib/commands/transfer/transfer.js b/packages/core/strapi/lib/commands/transfer/transfer.js index 4c0743f8d4..c8a4b530d3 100644 --- a/packages/core/strapi/lib/commands/transfer/transfer.js +++ b/packages/core/strapi/lib/commands/transfer/transfer.js @@ -15,8 +15,8 @@ const { buildTransferTable, createStrapiInstance, DEFAULT_IGNORED_CONTENT_TYPES, + formatDiagnostic, } = require('./utils'); -const formatDiagnosticErrors = require('./utils'); const logger = console; @@ -110,7 +110,7 @@ module.exports = async (opts) => { }, }); - engine.diagnostics.onDiagnostic(formatDiagnosticErrors); + engine.diagnostics.onDiagnostic(formatDiagnostic); try { logger.log(`Starting transfer...`); diff --git a/packages/core/strapi/lib/commands/transfer/utils.js b/packages/core/strapi/lib/commands/transfer/utils.js index 083d371953..c89016012b 100644 --- a/packages/core/strapi/lib/commands/transfer/utils.js +++ b/packages/core/strapi/lib/commands/transfer/utils.js @@ -4,10 +4,11 @@ const chalk = require('chalk'); const Table = require('cli-table3'); const { Option } = require('commander'); const { TransferGroupPresets } = require('@strapi/data-transfer/lib/engine'); + const { - config: { createOutputFileConfiguration }, + configs: { createOutputFileConfiguration }, createLogger, -} = require('@strapi/logger'); +} = require('@strapi/logger/lib'); const { readableBytes, exitWith } = require('../utils/helpers'); const strapi = require('../../index'); const { getParseListWithChoices } = require('../utils/commander'); @@ -179,5 +180,5 @@ module.exports = { excludeOption, onlyOption, validateExcludeOnly, - formatDiagnosticErrors: formatDiagnostic, + formatDiagnostic, }; diff --git a/packages/utils/logger/lib/config/output-file-configuration.js b/packages/utils/logger/lib/config/output-file-configuration.js deleted file mode 100644 index 87f34ea026..0000000000 --- a/packages/utils/logger/lib/config/output-file-configuration.js +++ /dev/null @@ -1,31 +0,0 @@ -'use strict'; - -const { transports, format } = require('winston'); -const { LEVEL_LABEL, LEVELS } = require('../constants'); -const { prettyPrint } = require('../formats'); - -/** - * This will remove the chalk color codes from the message provided. - * It's used to log plain text in the log file - */ -const getPlainText = format.printf(({ message }) => { - return message.replace( - // eslint-disable-next-line no-control-regex - /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g, - '' - ); -}); - -const createOutputFileConfiguration = (filename) => { - return { - level: LEVEL_LABEL, - levels: LEVELS, - format: prettyPrint(), - transports: [ - new transports.Console(), - new transports.File({ level: 'error', filename, format: getPlainText }), - ], - }; -}; - -module.exports = createOutputFileConfiguration; diff --git a/packages/utils/logger/lib/config/default-configuration.js b/packages/utils/logger/lib/configs/default-configuration.js similarity index 100% rename from packages/utils/logger/lib/config/default-configuration.js rename to packages/utils/logger/lib/configs/default-configuration.js diff --git a/packages/utils/logger/lib/configs/index.js b/packages/utils/logger/lib/configs/index.js new file mode 100644 index 0000000000..5490935f05 --- /dev/null +++ b/packages/utils/logger/lib/configs/index.js @@ -0,0 +1,9 @@ +'use strict'; + +const createDefaultConfiguration = require('./default-configuration'); +const createOutputFileConfiguration = require('./output-file-configuration'); + +module.exports = { + createDefaultConfiguration, + createOutputFileConfiguration, +}; diff --git a/packages/utils/logger/lib/configs/output-file-configuration.js b/packages/utils/logger/lib/configs/output-file-configuration.js new file mode 100644 index 0000000000..20139c6e2e --- /dev/null +++ b/packages/utils/logger/lib/configs/output-file-configuration.js @@ -0,0 +1,20 @@ +'use strict'; + +const { transports } = require('winston'); +const { LEVEL_LABEL, LEVELS } = require('../constants'); +const { prettyPrint } = require('../formats'); +const excludeColors = require('../formats/exclude-colors'); + +const createOutputFileConfiguration = (filename) => { + return { + level: LEVEL_LABEL, + levels: LEVELS, + format: prettyPrint(), + transports: [ + new transports.Console(), + new transports.File({ level: 'error', filename, format: excludeColors }), + ], + }; +}; + +module.exports = createOutputFileConfiguration; diff --git a/packages/utils/logger/lib/formats/exclude-colors.js b/packages/utils/logger/lib/formats/exclude-colors.js new file mode 100644 index 0000000000..70ba41ae48 --- /dev/null +++ b/packages/utils/logger/lib/formats/exclude-colors.js @@ -0,0 +1,17 @@ +'use strict'; + +const { format } = require('winston'); + +/** + * This will remove the chalk color codes from the message provided. + * It's used to log plain text in the log file + */ +const excludeColors = format.printf(({ message }) => { + return message.replace( + // eslint-disable-next-line no-control-regex + /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g, + '' + ); +}); + +module.exports = excludeColors; diff --git a/packages/utils/logger/lib/index.js b/packages/utils/logger/lib/index.js index 1077dbeb14..f7de8d88bf 100644 --- a/packages/utils/logger/lib/index.js +++ b/packages/utils/logger/lib/index.js @@ -3,11 +3,10 @@ const winston = require('winston'); const formats = require('./formats'); -const createDefaultConfiguration = require('./config/default-configuration'); -const createOutputFileConfiguration = require('./config/output-file-configuration'); +const configs = require('./configs'); const createLogger = (userConfiguration = {}) => { - const configuration = createDefaultConfiguration(); + const configuration = configs.createDefaultConfiguration(); Object.assign(configuration, userConfiguration); @@ -18,5 +17,5 @@ module.exports = { createLogger, winston, formats, - config: { createDefaultConfiguration, createOutputFileConfiguration }, + configs, }; From a68350ace055cbd464705147bb2240546c3c8253 Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Tue, 24 Jan 2023 11:11:26 +0100 Subject: [PATCH 11/16] Abstract format to index --- packages/utils/logger/lib/configs/output-file-configuration.js | 2 +- packages/utils/logger/lib/formats/index.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/utils/logger/lib/configs/output-file-configuration.js b/packages/utils/logger/lib/configs/output-file-configuration.js index 20139c6e2e..e78a6939f7 100644 --- a/packages/utils/logger/lib/configs/output-file-configuration.js +++ b/packages/utils/logger/lib/configs/output-file-configuration.js @@ -3,7 +3,7 @@ const { transports } = require('winston'); const { LEVEL_LABEL, LEVELS } = require('../constants'); const { prettyPrint } = require('../formats'); -const excludeColors = require('../formats/exclude-colors'); +const { excludeColors } = require('../formats'); const createOutputFileConfiguration = (filename) => { return { diff --git a/packages/utils/logger/lib/formats/index.js b/packages/utils/logger/lib/formats/index.js index e23b1ef747..6a2c9ddde3 100644 --- a/packages/utils/logger/lib/formats/index.js +++ b/packages/utils/logger/lib/formats/index.js @@ -2,8 +2,10 @@ const prettyPrint = require('./pretty-print'); const levelFilter = require('./level-filter'); +const excludeColors = require('./exclude-colors'); module.exports = { prettyPrint, levelFilter, + excludeColors, }; From 56528817e28a165040df96cf756253af7582a68f Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Tue, 24 Jan 2023 13:34:42 +0100 Subject: [PATCH 12/16] Fix tests --- .../src/strapi/providers/local-destination/index.ts | 6 +++--- .../src/strapi/providers/local-source/index.ts | 6 +++--- packages/core/data-transfer/src/utils/providers.ts | 2 +- .../lib/commands/__tests__/data-transfer/export.test.js | 6 +++++- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts b/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts index a8bd6eee24..a4afe549c8 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-destination/index.ts @@ -92,7 +92,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } getSchemas() { - assertValidStrapi(this.strapi, 'Not able to get Schemas.'); + assertValidStrapi(this.strapi, 'Not able to get Schemas'); const schemas = { ...this.strapi.contentTypes, ...this.strapi.components, @@ -129,7 +129,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { // TODO: Move this logic to the restore strategy async createAssetsWriteStream(): Promise { - assertValidStrapi(this.strapi, 'Not able to stream Assets.'); + assertValidStrapi(this.strapi, 'Not able to stream Assets'); const assetsDirectory = path.join(this.strapi.dirs.static.public, 'uploads'); const backupDirectory = path.join( @@ -175,7 +175,7 @@ class LocalStrapiDestinationProvider implements IDestinationProvider { } async createConfigurationWriteStream(): Promise { - assertValidStrapi(this.strapi, 'Not able to stream Configurations.'); + assertValidStrapi(this.strapi, 'Not able to stream Configurations'); const { strategy } = this.options; diff --git a/packages/core/data-transfer/src/strapi/providers/local-source/index.ts b/packages/core/data-transfer/src/strapi/providers/local-source/index.ts index b4e928eaa8..b1401c967f 100644 --- a/packages/core/data-transfer/src/strapi/providers/local-source/index.ts +++ b/packages/core/data-transfer/src/strapi/providers/local-source/index.ts @@ -65,7 +65,7 @@ class LocalStrapiSourceProvider implements ISourceProvider { } async createEntitiesReadStream(): Promise { - assertValidStrapi(this.strapi, 'Not able to stream entities.'); + assertValidStrapi(this.strapi, 'Not able to stream entities'); return chain([ // Entities stream @@ -77,13 +77,13 @@ class LocalStrapiSourceProvider implements ISourceProvider { } createLinksReadStream(): Readable { - assertValidStrapi(this.strapi, 'Not able to stream links.'); + assertValidStrapi(this.strapi, 'Not able to stream links'); return createLinksStream(this.strapi); } createConfigurationReadStream(): Readable { - assertValidStrapi(this.strapi, 'Not able to stream configuration. Strapi instance not found'); + assertValidStrapi(this.strapi, 'Not able to stream configuration'); return createConfigurationStream(strapi); } diff --git a/packages/core/data-transfer/src/utils/providers.ts b/packages/core/data-transfer/src/utils/providers.ts index a93d84a8d5..8d7df22a41 100644 --- a/packages/core/data-transfer/src/utils/providers.ts +++ b/packages/core/data-transfer/src/utils/providers.ts @@ -10,6 +10,6 @@ export const assertValidStrapi: ValidStrapiAssertion = ( msg = '' ): asserts strapi => { if (!strapi) { - throw new ProviderInitializationError(`Strapi instance not found. ${msg}`); + throw new ProviderInitializationError(`${msg}. Strapi instance not found.`); } }; diff --git a/packages/core/strapi/lib/commands/__tests__/data-transfer/export.test.js b/packages/core/strapi/lib/commands/__tests__/data-transfer/export.test.js index 06afadc740..05e0cc620d 100644 --- a/packages/core/strapi/lib/commands/__tests__/data-transfer/export.test.js +++ b/packages/core/strapi/lib/commands/__tests__/data-transfer/export.test.js @@ -26,7 +26,10 @@ describe('Export', () => { }, sourceProvider: { name: 'testSource' }, destinationProvider: { name: 'testDestination' }, - diagnostics: { on: jest.fn().mockReturnThis() }, + diagnostics: { + on: jest.fn().mockReturnThis(), + onDiagnostic: jest.fn().mockReturnThis(), + }, }; }, }, @@ -38,6 +41,7 @@ describe('Export', () => { // mock utils const mockUtils = { + formatDiagnostic: jest.fn(), createStrapiInstance() { return { telemetry: { From 9e19aed7fcf78e539861b2387b233d3800fd4092 Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Tue, 24 Jan 2023 13:47:41 +0100 Subject: [PATCH 13/16] Remove unnecessary typing --- packages/core/data-transfer/src/utils/providers.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/core/data-transfer/src/utils/providers.ts b/packages/core/data-transfer/src/utils/providers.ts index 8d7df22a41..a180d74aa2 100644 --- a/packages/core/data-transfer/src/utils/providers.ts +++ b/packages/core/data-transfer/src/utils/providers.ts @@ -5,10 +5,7 @@ export type ValidStrapiAssertion = ( msg?: string ) => asserts strapi is Strapi.Strapi; -export const assertValidStrapi: ValidStrapiAssertion = ( - strapi?: unknown, - msg = '' -): asserts strapi => { +export const assertValidStrapi: ValidStrapiAssertion = (strapi?: unknown, msg = '') => { if (!strapi) { throw new ProviderInitializationError(`${msg}. Strapi instance not found.`); } From fb850a43dc9386c27709a5f8eb2a739f36da6bef Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Tue, 24 Jan 2023 14:52:03 +0100 Subject: [PATCH 14/16] Remove unnecessary part of import --- packages/core/strapi/lib/commands/transfer/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/strapi/lib/commands/transfer/utils.js b/packages/core/strapi/lib/commands/transfer/utils.js index f1abec7616..6f0b14f5ef 100644 --- a/packages/core/strapi/lib/commands/transfer/utils.js +++ b/packages/core/strapi/lib/commands/transfer/utils.js @@ -8,7 +8,7 @@ const { TransferGroupPresets } = require('@strapi/data-transfer/lib/engine'); const { configs: { createOutputFileConfiguration }, createLogger, -} = require('@strapi/logger/lib'); +} = require('@strapi/logger'); const { readableBytes, exitWith } = require('../utils/helpers'); const strapi = require('../../index'); const { getParseListWithChoices } = require('../utils/commander'); From 5f8422140e27af47f92e8a02a51445d2dce1f99f Mon Sep 17 00:00:00 2001 From: Christian Capeans Date: Tue, 24 Jan 2023 15:54:16 +0100 Subject: [PATCH 15/16] Add operation to the format diagnostic functino --- .../strapi/lib/commands/transfer/export.js | 2 +- .../strapi/lib/commands/transfer/import.js | 2 +- .../strapi/lib/commands/transfer/transfer.js | 2 +- .../strapi/lib/commands/transfer/utils.js | 71 ++++++++++--------- 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/packages/core/strapi/lib/commands/transfer/export.js b/packages/core/strapi/lib/commands/transfer/export.js index 9bdf88773d..6197a8e062 100644 --- a/packages/core/strapi/lib/commands/transfer/export.js +++ b/packages/core/strapi/lib/commands/transfer/export.js @@ -78,7 +78,7 @@ module.exports = async (opts) => { }, }); - engine.diagnostics.onDiagnostic(formatDiagnostic); + engine.diagnostics.onDiagnostic(formatDiagnostic('export')); const progress = engine.progress.stream; diff --git a/packages/core/strapi/lib/commands/transfer/import.js b/packages/core/strapi/lib/commands/transfer/import.js index 652e54eac9..b2dbc96bf1 100644 --- a/packages/core/strapi/lib/commands/transfer/import.js +++ b/packages/core/strapi/lib/commands/transfer/import.js @@ -87,7 +87,7 @@ module.exports = async (opts) => { const engine = createTransferEngine(source, destination, engineOptions); - engine.diagnostics.onDiagnostic(formatDiagnostic); + engine.diagnostics.onDiagnostic(formatDiagnostic('import')); const progress = engine.progress.stream; const getTelemetryPayload = () => { diff --git a/packages/core/strapi/lib/commands/transfer/transfer.js b/packages/core/strapi/lib/commands/transfer/transfer.js index c8a4b530d3..2c79057e66 100644 --- a/packages/core/strapi/lib/commands/transfer/transfer.js +++ b/packages/core/strapi/lib/commands/transfer/transfer.js @@ -110,7 +110,7 @@ module.exports = async (opts) => { }, }); - engine.diagnostics.onDiagnostic(formatDiagnostic); + engine.diagnostics.onDiagnostic(formatDiagnostic('transfer')); try { logger.log(`Starting transfer...`); diff --git a/packages/core/strapi/lib/commands/transfer/utils.js b/packages/core/strapi/lib/commands/transfer/utils.js index 6f0b14f5ef..f0a8dee0fa 100644 --- a/packages/core/strapi/lib/commands/transfer/utils.js +++ b/packages/core/strapi/lib/commands/transfer/utils.js @@ -126,51 +126,54 @@ const validateExcludeOnly = (command) => { } }; -const logger = createLogger(createOutputFileConfiguration(`import_error_log_${Date.now()}.log`)); - const errorColors = { fatal: chalk.red, error: chalk.red, silly: chalk.yellow, }; -const formatDiagnostic = ({ details, kind }) => { - try { - if (kind === 'error') { - const { message, severity = 'fatal', error, details: moreDetails } = details; +const formatDiagnostic = + (operation) => + ({ details, kind }) => { + const logger = createLogger( + createOutputFileConfiguration(`${operation}_error_log_${Date.now()}.log`) + ); + try { + if (kind === 'error') { + const { message, severity = 'fatal', error, details: moreDetails } = details; - const detailsInfo = error ?? moreDetails; - let errorMessage = errorColors[severity](`[${severity.toUpperCase()}] ${message}`); - if (detailsInfo && detailsInfo.details) { - const { - origin, - details: { step, details: stepDetails, ...moreInfo }, - } = detailsInfo; - errorMessage = `${errorMessage}. Thrown at ${origin} during ${step}.\n`; - if (stepDetails || moreInfo) { - const { check, ...info } = stepDetails ?? moreInfo; - errorMessage = `${errorMessage} Check ${check ?? ''}: ${JSON.stringify(info, null, 2)}`; + const detailsInfo = error ?? moreDetails; + let errorMessage = errorColors[severity](`[${severity.toUpperCase()}] ${message}`); + if (detailsInfo && detailsInfo.details) { + const { + origin, + details: { step, details: stepDetails, ...moreInfo }, + } = detailsInfo; + errorMessage = `${errorMessage}. Thrown at ${origin} during ${step}.\n`; + if (stepDetails || moreInfo) { + const { check, ...info } = stepDetails ?? moreInfo; + errorMessage = `${errorMessage} Check ${check ?? ''}: ${JSON.stringify(info, null, 2)}`; + } } + + logger.error(new Error(errorMessage, error)); } + if (kind === 'info') { + const { message, params } = details; - logger.error(new Error(errorMessage, error)); + const msg = `${message}\n${params ? JSON.stringify(params, null, 2) : ''}`; + + logger.info(msg); + } + if (kind === 'warning') { + const { origin, message } = details; + + logger.warn(`(${origin ?? 'transfer'}) ${message}`); + } + } catch (err) { + logger.error(err); } - if (kind === 'info') { - const { message, params } = details; - - const msg = `${message}\n${params ? JSON.stringify(params, null, 2) : ''}`; - - logger.info(msg); - } - if (kind === 'warning') { - const { origin, message } = details; - - logger.warn(`(${origin ?? 'transfer'}) ${message}`); - } - } catch (err) { - logger.error(err); - } -}; + }; module.exports = { buildTransferTable, From 80c0153907dab0b04b541d6570dffba2cb159c50 Mon Sep 17 00:00:00 2001 From: Convly Date: Tue, 24 Jan 2023 16:15:55 +0100 Subject: [PATCH 16/16] Fix diff formatting for schemas matching --- packages/core/data-transfer/src/engine/index.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/core/data-transfer/src/engine/index.ts b/packages/core/data-transfer/src/engine/index.ts index 9b89e084dc..5d04e6f932 100644 --- a/packages/core/data-transfer/src/engine/index.ts +++ b/packages/core/data-transfer/src/engine/index.ts @@ -346,7 +346,7 @@ class TransferEngine< keys.forEach((key) => { const sourceSchema = sourceSchemas[key]; const destinationSchema = destinationSchemas[key]; - const schemaDiffs = compareSchemas(sourceSchema, destinationSchema, strategy); + const schemaDiffs = compareSchemas(destinationSchema, sourceSchema, strategy); if (schemaDiffs.length) { diffs[key] = schemaDiffs; @@ -361,16 +361,18 @@ class TransferEngine< msg += ctDiffs .sort((a, b) => (a.kind > b.kind ? -1 : 1)) .map((diff) => { + const path = diff.path.join('.'); + if (diff.kind === 'added') { - return `Added "${diff.path}": "${diff.value}" (${diff.type})`; + return `Added "${path}": "${diff.value}" (${diff.type})`; } if (diff.kind === 'deleted') { - return `Removed "${diff.path}"`; + return `Removed "${path}"`; } if (diff.kind === 'modified') { - return `Modified "${diff.path}". "${diff.values[0]}" (${diff.types[0]}) => "${diff.values[1]}" (${diff.types[1]})`; + return `Modified "${path}": "${diff.values[0]}" (${diff.types[0]}) => "${diff.values[1]}" (${diff.types[1]})`; } throw new TransferEngineValidationError(`Invalid diff found for "${uid}"`, {