Merge pull request #2446 from micalevisk/fix/issue-2434

fix: exit with an error when some config file was found but lacks on reading permissions

lib/compiler/assets-manager.ts

@@ -101,8 +101,9 @@ export class AssetsManager {
 
           this.watchers.push(watcher);
         } else {
-          const files = sync(item.glob, { ignore: item.exclude })
-            .filter((matched) => statSync(matched).isFile());
+          const files = sync(item.glob, { ignore: item.exclude }).filter(
+            (matched) => statSync(matched).isFile(),
+          );
           for (const path of files) {
             this.actionOnFile({ ...option, path, action: 'change' });
           }

lib/configuration/nest-configuration.loader.ts

@@ -1,4 +1,4 @@
-import { Reader } from '../readers';
+import { Reader, ReaderFileLackPersmissionsError } from '../readers';
 import { Configuration } from './configuration';
 import { ConfigurationLoader } from './configuration.loader';
 import { defaultConfiguration } from './defaults';
@@ -14,7 +14,7 @@ const loadedConfigsCache = new Map<string, Required<Configuration>>();
 export class NestConfigurationLoader implements ConfigurationLoader {
   constructor(private readonly reader: Reader) {}
 
-  public async load(name?: string): Promise<Required<Configuration>> {
+  public load(name?: string): Required<Configuration> {
     const cacheEntryKey = `${this.reader.constructor.name}:${name}`;
     const cachedConfig = loadedConfigsCache.get(cacheEntryKey);
     if (cachedConfig) {
@@ -23,17 +23,24 @@ export class NestConfigurationLoader implements ConfigurationLoader {
 
     let loadedConfig: Required<Configuration> | undefined;
 
-    const content: string | undefined = name
-      ? await this.reader.read(name)
-      : await this.reader.readAnyOf([
+    const contentOrError = name
+      ? this.reader.read(name)
+      : this.reader.readAnyOf([
           'nest-cli.json',
           '.nestcli.json',
           '.nest-cli.json',
           'nest.json',
         ]);
 
-    if (content) {
-      const fileConfig = JSON.parse(content);
+    if (contentOrError) {
+      const isMissingPersmissionsError =
+        contentOrError instanceof ReaderFileLackPersmissionsError;
+      if (isMissingPersmissionsError) {
+        console.error(contentOrError.message);
+        process.exit(1);
+      }
+
+      const fileConfig = JSON.parse(contentOrError);
       if (fileConfig.compilerOptions) {
         loadedConfig = {
           ...defaultConfiguration,

lib/readers/file-system.reader.ts

@@ -1,27 +1,54 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import { Reader } from './reader';
+import { Reader, ReaderFileLackPersmissionsError } from './reader';
 
 export class FileSystemReader implements Reader {
   constructor(private readonly directory: string) {}
 
-  public list(): Promise<string[]> {
-    return fs.promises.readdir(this.directory);
+  public list(): string[] {
+    return fs.readdirSync(this.directory);
   }
 
-  public read(name: string): Promise<string> {
-    return fs.promises.readFile(path.join(this.directory, name), 'utf8');
+  public read(name: string): string {
+    return fs.readFileSync(path.join(this.directory, name), 'utf8');
   }
 
-  public async readAnyOf(filenames: string[]): Promise<string | undefined> {
-    try {
-      for (const file of filenames) {
-        return await this.read(file);
+  public readAnyOf(
+    filenames: string[],
+  ): string | undefined | ReaderFileLackPersmissionsError {
+    let firstFilePathFoundButWithInsufficientPermissions: string | undefined;
+
+    for (let id = 0; id < filenames.length; id++) {
+      const file = filenames[id];
+
+      try {
+        return this.read(file);
+      } catch (readErr) {
+        if (
+          !firstFilePathFoundButWithInsufficientPermissions &&
+          typeof readErr?.code === 'string'
+        ) {
+          const isInsufficientPermissionsError =
+            readErr.code === 'EACCES' || readErr.code === 'EPERM';
+          if (isInsufficientPermissionsError) {
+            firstFilePathFoundButWithInsufficientPermissions = readErr.path;
+          }
+        }
+
+        const isLastFileToLookFor = id === filenames.length - 1;
+        if (!isLastFileToLookFor) {
+          continue;
+        }
+
+        if (firstFilePathFoundButWithInsufficientPermissions) {
+          return new ReaderFileLackPersmissionsError(
+            firstFilePathFoundButWithInsufficientPermissions,
+            readErr.code,
+          );
+        } else {
+          return undefined;
+        }
       }
-    } catch (err) {
-      return filenames.length > 0
-        ? await this.readAnyOf(filenames.slice(1, filenames.length))
-        : undefined;
     }
   }
 }

lib/readers/reader.ts

@@ -1,5 +1,16 @@
+export class ReaderFileLackPersmissionsError extends Error {
+  constructor(
+    public readonly filePath: string,
+    public readonly fsErrorCode: string,
+  ) {
+    super(`File ${filePath} lacks read permissions!`);
+  }
+}
+
 export interface Reader {
-  list(): string[] | Promise<string[]>;
-  read(name: string): string | Promise<string>;
-  readAnyOf(filenames: string[]): string | Promise<string | undefined>;
+  list(): string[];
+  read(name: string): string;
+  readAnyOf(
+    filenames: string[],
+  ): string | undefined | ReaderFileLackPersmissionsError;
 }

test/lib/configuration/nest-configuration.loader.spec.ts

@@ -9,21 +9,17 @@ describe('Nest Configuration Loader', () => {
     mock.mockImplementation(() => {
       return {
         readAnyOf: jest.fn(() =>
-          Promise.resolve(
-            JSON.stringify({
-              language: 'ts',
-              collection: '@nestjs/schematics',
-            }),
-          ),
+          JSON.stringify({
+            language: 'ts',
+            collection: '@nestjs/schematics',
+          }),
         ),
         read: jest.fn(() =>
-          Promise.resolve(
-            JSON.stringify({
-              language: 'ts',
-              collection: '@nestjs/schematics',
-              entryFile: 'secondary',
-            }),
-          ),
+          JSON.stringify({
+            language: 'ts',
+            collection: '@nestjs/schematics',
+            entryFile: 'secondary',
+          }),
         ),
       };
     });

test/lib/readers/file-system.reader.spec.ts

@@ -2,10 +2,8 @@ import * as fs from 'fs';
 import { FileSystemReader, Reader } from '../../../lib/readers';
 
 jest.mock('fs', () => ({
-  promises: {
-    readdir: jest.fn().mockResolvedValue([]),
-    readFile: jest.fn().mockResolvedValue('content'),
-  },
+  readdirSync: jest.fn().mockResolvedValue([]),
+  readFileSync: jest.fn().mockResolvedValue('content'),
 }));
 
 const dir: string = process.cwd();
@@ -15,25 +13,25 @@ describe('File System Reader', () => {
   afterAll(() => {
     jest.clearAllMocks();
   });
-  it('should use fs.promises.readdir when list', async () => {
-    await reader.list();
-    expect(fs.promises.readdir).toHaveBeenCalled();
+  it('should use fs.readdirSync when list (for performance reasons)', async () => {
+    reader.list();
+    expect(fs.readdirSync).toHaveBeenCalled();
   });
-  it('should use fs.promises.readFile when read', async () => {
-    await reader.read('filename');
-    expect(fs.promises.readFile).toHaveBeenCalled();
+  it('should use fs.readFileSync when read (for performance reasons)', async () => {
+    reader.read('filename');
+    expect(fs.readFileSync).toHaveBeenCalled();
   });
 
   describe('readAnyOf tests', () => {
-    it('should call readFile when running readAnyOf fn', async () => {
+    it('should call readFileSync when running readAnyOf fn', async () => {
       const filenames: string[] = ['file1', 'file2', 'file3'];
-      await reader.readAnyOf(filenames);
+      reader.readAnyOf(filenames);
 
-      expect(fs.promises.readFile).toHaveBeenCalled();
+      expect(fs.readFileSync).toHaveBeenCalled();
     });
 
     it('should return undefined when no file is passed', async () => {
-      const content = await reader.readAnyOf([]);
+      const content = reader.readAnyOf([]);
       expect(content).toEqual(undefined);
     });
   });