Merge ea9cfba627 into 1d67af1558

playground/__tests__/cli.test.js

@@ -1,5 +1,6 @@
 'use strict';
 
+const fs = require('fs');
 const util = require('util');
 const exec = util.promisify(require('child_process').exec);
 
@@ -46,6 +47,27 @@ describe('Test the config-sync CLI', () => {
     expect(diffOutput).toContain('No differences between DB and sync directory');
   });
 
+  test('Import (unparsable file)', async () => {
+    // A file that can't be parsed (a merge conflict left in it, a typo) is not a missing file:
+    // importing it must fail, and leave the config in the database.
+    const file = 'config/sync/user-role.public.json';
+    const original = fs.readFileSync(file, 'utf8');
+    fs.writeFileSync(file, '<<<<<<< HEAD\n{ "name": "Public",\n');
+    let output;
+    try {
+      const { stdout, stderr } = await exec('yarn cs import -p user-role.public -y');
+      output = stdout + stderr;
+    } catch (e) {
+      output = `${e.stdout}${e.stderr}`;
+    } finally {
+      fs.writeFileSync(file, original);
+    }
+    expect(output).toContain('is not valid JSON');
+    // The role is still in the database: with its file back, nothing differs.
+    const { stdout: diffOutput } = await exec('yarn cs diff');
+    expect(diffOutput).toContain('No differences between DB and sync directory');
+  });
+
   test('Non-empty diff returns 1', async () => {
     await exec('rm -rf config/sync/admin-role.strapi-editor.json');
     // Work around Jest not supporting custom error matching.

server/services/main.js

@@ -94,14 +94,24 @@ export default () => ({
     // Replace reserved characters in filenames.
     configName = configName.replace(/:/g, "#").replace(/\//g, "$");
 
+    const filePath = `${strapi.config.get('plugin::config-sync.syncDir')}${configType}.${configName}.json`;
     const readFile = util.promisify(fs.readFile);
-    return readFile(`${strapi.config.get('plugin::config-sync.syncDir')}${configType}.${configName}.json`)
+
-      .then((data) => {
+    let data;
+    try {
+      data = await readFile(filePath);
+    } catch (e) {
+      // No file for this config.
+      if (e.code === 'ENOENT') return null;
+      throw e;
+    }
+
+    try {
       return JSON.parse(data);
-      })
+    } catch (e) {
-      .catch(() => {
+      // A file that can't be parsed is not a missing file: it must never lead to deleting the config.
-        return null;
+      throw new Error(`The config file '${filePath}' is not valid JSON (${e.message}).`);
-      });
+    }
   },
 
 
@@ -136,7 +146,13 @@ export default () => ({
           return;
         }
 
-        const fileContents = await strapi.plugin('config-sync').service('main').readConfigFile(type, name);
+        let fileContents;
+        try {
+          fileContents = await strapi.plugin('config-sync').service('main').readConfigFile(type, name);
+        } catch (e) {
+          strapi.log.warn(logMessage(`The config file '${file}' can't be read and is left out of the diff: ${e.message}`));
+          return;
+        }
 
         if (!fileContents) {
           strapi.log.warn(logMessage(`An empty config file '${file}' was found in the sync directory`));
@@ -242,9 +258,9 @@ export default () => ({
 
     const type = configName.split('.')[0]; // Grab the first part of the filename.
     const name = configName.split(/\.(.+)/)[1]; // Grab the rest of the filename.
-    const fileContents = await strapi.plugin('config-sync').service('main').readConfigFile(type, name);
 
     try {
+      const fileContents = await strapi.plugin('config-sync').service('main').readConfigFile(type, name);
       const importState = await strapi.plugin('config-sync').types[type].importSingle(name, fileContents, force);
       if (onSuccess && importState !== false) onSuccess(`${type}.${name}`);
     } catch (e) {