Merge pull request #98 from tractr/next

feat(cli): improve errors logging

packages/cli/src/lib/commands/push.ts

@@ -1,6 +1,11 @@
 import { Container } from 'typedi';
 import pino from 'pino';
-import { ConfigService, MigrationClient, SnapshotClient } from '../services';
+import {
+  ConfigService,
+  MigrationClient,
+  PingClient,
+  SnapshotClient,
+} from '../services';
 import { loadCollections } from '../loader';
 import { LOGGER } from '../constants';
 
@@ -16,6 +21,10 @@ export async function runPush() {
 
   // Snapshot
   if (snapshotConfig.enabled) {
+    // Test if the directus extension is installed
+    // At this point, it could be not installed and cause an issue with the snapshot push
+    await Container.get(PingClient).test();
+
     logger.info(`---- Push schema ----`);
     await Container.get(SnapshotClient).push();
   } else {

packages/cli/src/lib/loader.ts

@@ -26,18 +26,18 @@ export async function initContext(
   programOptions: object,
   commandOptions: object,
 ) {
-  // Set temporary logger, in case of error when loading the config
-  Container.set(
-    LOGGER,
-    Logger({
-      transport: getPinoTransport(),
-      level: 'error',
-    }),
-  );
+  // Set temporary logger. This allows the config process to log infos and errors
+  const tempLogger = Logger({
+    transport: getPinoTransport(),
+    level: 'info',
+  });
+  Container.set(LOGGER, tempLogger);
   // Get the config service
   const config = Container.get(ConfigService);
   // Set the options
   config.setOptions(programOptions, commandOptions);
+  // Flush previous logs
+  tempLogger.flush();
   // Define the logger
   Container.set(
     LOGGER,

packages/cli/src/lib/services/collections/base/id-mapper-client.ts

@@ -1,5 +1,5 @@
 import { MigrationClient } from '../../migration-client';
-import { ExtensionClient } from '../../extension-client';
+import { ExtensionClient, NO_ID_MAP_MESSAGE } from '../../extension-client';
 import pino from 'pino';
 
 export interface IdMap {
@@ -40,7 +40,7 @@ export abstract class IdMapperClient extends ExtensionClient {
     const idMap = await this.fetch<IdMap>(
       `/table/${this.table}/sync_id/${syncId}`,
     ).catch((error) => {
-      if (error.message === 'No id map found') {
+      if (error.message === NO_ID_MAP_MESSAGE) {
         return undefined;
       }
       throw error;
@@ -62,7 +62,7 @@ export abstract class IdMapperClient extends ExtensionClient {
     const idMap = await this.fetch<IdMap>(
       `/table/${this.table}/local_id/${localId}`,
     ).catch((error) => {
-      if (error.message === 'No id map found') {
+      if (error.message === NO_ID_MAP_MESSAGE) {
         return undefined;
       }
       throw error;

packages/cli/src/lib/services/config/config.ts

@@ -1,22 +1,24 @@
-import { Service } from 'typedi';
+import { Container, Service } from 'typedi';
 import {
+  CollectionHooks,
+  CollectionName,
+  CollectionPreservableIdName,
   ConfigFileOptions,
   DirectusConfigWithCredentials,
   DirectusConfigWithToken,
-  CollectionName,
-  CollectionHooks,
-  SnapshotHooks,
   OptionName,
   Options,
-  CollectionPreservableIdName,
+  SnapshotHooks,
 } from './interfaces';
 import Path from 'path';
 import { Cacheable } from 'typescript-cacheable';
 import { ConfigFileLoader } from './config-file-loader';
-import { zodParse } from '../../helpers';
+import { getChildLogger, zodParse } from '../../helpers';
 import deepmerge from 'deepmerge';
 import { DefaultConfig, DefaultConfigPaths } from './default-config';
 import { CollectionsList, OptionsSchema } from './schema';
+import { LOGGER } from '../../constants';
+import pino from 'pino';
 
 @Service()
 export class ConfigService {
@@ -185,6 +187,7 @@ export class ConfigService {
 
   @Cacheable()
   protected getFileOptions(): ConfigFileOptions | undefined {
+    const logger = this.getLogger();
     const customConfigPath = this.programOptions?.configPath;
     const possiblePaths = [
       ...(customConfigPath ? [Path.resolve(customConfigPath)] : []),
@@ -195,10 +198,22 @@ export class ConfigService {
     for (const configPath of possiblePaths) {
       const config = new ConfigFileLoader(configPath).get();
       if (config) {
+        logger.info(`Loaded config file from ${configPath}`);
         return config;
       }
     }
-
+    logger.info(
+      `No config file found. Tried path: \n- ${possiblePaths.join('\n- ')}`,
+    );
     return undefined;
   }
+
+  /**
+   * Returns a temporary logger as it may be changed by another one with specific options
+   * See loader.ts file for more information
+   */
+  protected getLogger() {
+    const baseLogger = Container.get<pino.Logger>(LOGGER);
+    return getChildLogger(baseLogger, 'config');
+  }
 }

packages/cli/src/lib/services/extension-client.ts

@@ -2,6 +2,8 @@ import createHttpError from 'http-errors';
 import { MigrationClient } from './migration-client';
 import { Cacheable } from 'typescript-cacheable';
 
+export const NO_ID_MAP_MESSAGE = 'No id map found';
+
 /**
  * This class provides an interface to interact with the Directus extension sync endpoints.
  */

packages/cli/src/lib/services/index.ts

@@ -1,6 +1,7 @@
 export * from './config';
 export * from './migration-client';
 export * from './extension-client';
+export * from './ping-client';
 export * from './helpers-client';
 export * from './snapshot';
 export * from './specifications';

packages/cli/src/lib/services/ping-client.ts

@@ -0,0 +1,30 @@
+import { ExtensionClient, NO_ID_MAP_MESSAGE } from './extension-client';
+import { MigrationClient } from './migration-client';
+import { Service } from 'typedi';
+
+@Service()
+export class PingClient extends ExtensionClient {
+  constructor(migrationClient: MigrationClient) {
+    super(migrationClient);
+  }
+
+  /** Try to get a fake id map in order to check if the server is up and the extension is installed */
+  async test() {
+    const response = await this.fetch<unknown>(
+      `/table/__ping_test__/sync_id/__ping_test__`,
+      'GET',
+    )
+      .then(() => ({ success: true }))
+      .catch((error: Error) => ({ error }));
+
+    if ('success' in response) {
+      return; // Should not happen
+    }
+
+    if (response.error.message === NO_ID_MAP_MESSAGE) {
+      return;
+    }
+
+    throw response.error;
+  }
+}

packages/e2e/README.md

@@ -10,3 +10,25 @@ Therefore, you need to have a the dependencies installed in the host machine.
 You must install the following libraries:
 
 - `libvips`: `sudo apt install libvips` (https://libvips.github.io/libvips/install.html)
+
+## Running the tests
+
+First, you need to build the CLI and the extension. Go to the root of the repository and run:
+
+```bash
+npm run build
+```
+
+Then, you can run the tests:
+
+```bash
+npm run test
+```
+
+If you have edited the extension or the CLI, you need to rebuild them.
+
+### Running a single test
+
+By design, the tests are ran sequentially. This is done to avoid conflicts between Directus servers.
+
+To run a single test, you need to edit the test file and change the `it` function to `fit`.

packages/e2e/configs/config-path-info/directus-sync.config.cjs

@@ -0,0 +1,2 @@
+module.exports = {
+};

packages/e2e/package.json

@@ -11,8 +11,7 @@
     "directus": "directus",
     "directus-sync": "directus-sync",
     "bootstrap": "./bootstrap.sh",
-    "test": "rm -f ./logs/*.log && jasmine",
-    "build-test": "npm run build && npm run test",
+    "test": "npm run build && rm -f ./logs/*.log && jasmine",
     "upgrade": "zx upgrade-directus-version.mjs && zx update-dumps-from-current-version.mjs"
   },
   "keywords": [],

packages/e2e/spec/config/config-path-info.ts

@@ -0,0 +1,43 @@
+import { Context } from '../helpers/index.js';
+
+export const configPathInfo = (context: Context) => {
+  it('ensure logs show info about missing config file when wrong path is provided', async () => {
+    // Init sync client
+    const sync = await context.getSync(
+      'sources/empty-collections',
+      'wrong-path/directus-sync.config.cjs',
+    );
+
+    const output = await sync.diff();
+    const log = output.find((l) => l.msg.includes('No config file found'));
+
+    expect(log).toBeDefined();
+    expect(log?.msg).toContain('wrong-path/directus-sync.config.cjs');
+  });
+
+  it('ensure log shows info about missing config file for default files', async () => {
+    // Init sync client
+    const sync = await context.getSync('sources/empty-collections');
+
+    const output = await sync.diff();
+    const log = output.find((l) => l.msg.includes('No config file found'));
+
+    expect(log).toBeDefined();
+    expect(log?.msg).toContain('e2e/directus-sync.config.cjs');
+    expect(log?.msg).toContain('e2e/directus-sync.config.js');
+    expect(log?.msg).toContain('e2e/directus-sync.config.json');
+  });
+
+  it('ensure logs show info when config path exists', async () => {
+    // Init sync client
+    const sync = await context.getSync(
+      'sources/empty-collections',
+      'config-path-info/directus-sync.config.cjs',
+    );
+
+    const output = await sync.diff();
+    const log = output.find((l) => l.msg.includes('Loaded config file from'));
+
+    expect(log?.msg).toContain('config-path-info/directus-sync.config.cjs');
+  });
+};

packages/e2e/spec/config/index.ts

@@ -0,0 +1 @@
+export * from './config-path-info.js';

packages/e2e/spec/entrypoint.spec.ts

@@ -39,6 +39,7 @@ import {
   updateOperationsWithConflicts,
 } from './operations/index.js';
 import { updateDefaultData } from './default-data/index.js';
+import { configPathInfo } from './config/index.js';
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000;
 
@@ -92,6 +93,8 @@ describe('Tests entrypoint ->', () => {
   includeSomeCollections(context);
   noSnapshot(context);
 
+  configPathInfo(context);
+
   insertDuplicatedPermissions(context);
   removePermissionDuplicates(context);
   pullAndPushPublicPermissions(context);

packages/e2e/spec/helpers/sdk/context.ts

@@ -4,6 +4,7 @@ import fs from 'fs-extra';
 import { DirectusSync } from './directus-sync.js';
 import { SqliteClient } from './sqlite-client.js';
 import { sleep } from './async/index.js';
+import { DirectusSyncArgs } from './interfaces';
 
 const dumpBaseDirectory = Path.resolve('dumps');
 const configBaseDirectory = Path.resolve('configs');
@@ -45,21 +46,26 @@ export class Context {
     return this.instance;
   }
 
-  async getSync(dumpFolder: string, configPath?: string) {
+  async getSync(
+    dumpFolder: string,
+    configPath?: string,
+    factory: new (options: DirectusSyncArgs) => DirectusSync = DirectusSync,
+  ): Promise<DirectusSync> {
     const dumpPath = Path.resolve(dumpBaseDirectory, dumpFolder);
     const clearDumpPath = dumpFolder.startsWith('temp');
     if (clearDumpPath) {
       fs.rmSync(dumpPath, { recursive: true, force: true });
     }
     const directus = this.getDirectus();
     const instance = this.getInstance();
-    return new DirectusSync({
+    const options = {
       token: await directus.requireToken(),
       url: instance.getUrl(),
       dumpPath,
       configPath: configPath
         ? Path.resolve(configBaseDirectory, configPath)
         : undefined,
-    });
+    };
+    return new factory(options);
   }
 }

packages/e2e/spec/helpers/sdk/directus-sync.ts

@@ -123,16 +123,20 @@ export class DirectusSync {
   }
 
   protected getOptionsArgs(): string[] {
-    const requiredArgs = [
+    const requiredArgs = this.getRequiredArgs();
+    return this.options.configPath
+      ? [...requiredArgs, `--config-path`, this.options.configPath]
+      : requiredArgs;
+  }
+
+  protected getRequiredArgs(): string[] {
+    return [
       `--directus-token`,
       this.options.token,
       `--directus-url`,
       this.options.url,
       '--debug',
     ];
-    return this.options.configPath
-      ? [...requiredArgs, `--config-path`, this.options.configPath]
-      : requiredArgs;
   }
 
   protected getLogTransport(logFilePath: string): LoggerOptions['transport'] {