fix: 🐛 improve file opening and closing logic

src/fsa-to-node/FsaNodeCore.ts

@@ -4,6 +4,7 @@ import { ERRSTR } from '../node/constants';
 import { FsaToNodeConstants } from './constants';
 import { FsaNodeFsOpenFile } from './FsaNodeFsOpenFile';
 import { FLAG } from '../consts/FLAG';
+import * as util from '../node/util';
 import type * as fsa from '../fsa/types';
 import type * as misc from '../node/types/misc';
 import type { FsaNodeSyncAdapter } from './types';
@@ -81,10 +82,10 @@ export class FsaNodeCore {
   protected async getFileOrDir(
     path: string[],
     name: string,
-    funcName?: string,
-    create?: boolean,
+    funcName?: string
   ): Promise<fsa.IFileSystemFileHandle | fsa.IFileSystemDirectoryHandle> {
     const dir = await this.getDir(path, false, funcName);
+    if (!name) return dir;
     try {
       const file = await dir.getFileHandle(name);
       return file;
@@ -126,12 +127,11 @@ export class FsaNodeCore {
   public async __getFileById(
     id: misc.TFileId,
     funcName?: string,
-    create?: boolean,
   ): Promise<fsa.IFileSystemFileHandle> {
     if (typeof id === 'number') return (await this.getFileByFd(id, funcName)).file;
     const filename = pathToFilename(id);
     const [folder, name] = pathToLocation(filename);
-    return await this.getFile(folder, name, funcName, create);
+    return await this.getFile(folder, name, funcName);
   }
 
   protected async getFileByIdOrCreate(id: misc.TFileId, funcName?: string): Promise<fsa.IFileSystemFileHandle> {
@@ -144,9 +144,41 @@ export class FsaNodeCore {
 
   protected async __open(filename: string, flags: number, mode: number): Promise<FsaNodeFsOpenFile> {
     const [folder, name] = pathToLocation(filename);
-    const createIfMissing = !!(flags & FLAG.O_CREAT);
-    const fsaFile = await this.getFile(folder, name, 'open', createIfMissing);
-    return this.__open2(fsaFile, filename, flags, mode);
+    const throwIfExists = !!(flags & FLAG.O_EXCL);
+    if (throwIfExists) {
+      try {
+        await this.getFile(folder, name, 'open', false);
+        throw util.createError('EEXIST', 'writeFile');
+      } catch (error) {
+        const file404 = (error && typeof error === 'object' && (error.code === 'ENOENT' || error.name === 'NotFoundError'));
+        if (!file404) {
+          if (error && typeof error === 'object') {
+            switch (error.name) {
+              case 'TypeMismatchError':
+                throw createError('ENOTDIR', 'open', filename);
+              case 'NotFoundError':
+                throw createError('ENOENT', 'open', filename);
+            }
+          }
+          throw error;
+        }
+      }
+    }
+    try {
+      const createIfMissing = !!(flags & FLAG.O_CREAT);
+      const fsaFile = await this.getFile(folder, name, 'open', createIfMissing);
+      return this.__open2(fsaFile, filename, flags, mode);
+    } catch (error) {
+      if (error && typeof error === 'object') {
+        switch (error.name) {
+          case 'TypeMismatchError':
+            throw createError('ENOTDIR', 'open', filename);
+          case 'NotFoundError':
+            throw createError('ENOENT', 'open', filename);
+        }
+      }
+      throw error;
+    }
   }
 
   protected __open2(
@@ -161,6 +193,13 @@ export class FsaNodeCore {
     return file;
   }
 
+  protected async __close(fd: number): Promise<void> {
+    const openFile = await this.getFileByFdAsync(fd, 'close');
+    await openFile.close();
+    this.fds.delete(fd);
+    this.releasedFds.push(fd);
+  };
+
   protected getFileName(id: misc.TFileId): string {
     if (typeof id === 'number') {
       const openFile = this.fds.get(id);

src/fsa-to-node/FsaNodeFs.ts

@@ -63,18 +63,8 @@ export class FsaNodeFs extends FsaNodeCore implements FsCallbackApi, FsSynchrono
 
   public readonly close: FsCallbackApi['close'] = (fd: number, callback: misc.TCallback<void>): void => {
     util.validateFd(fd);
-    this.getFileByFdAsync(fd, 'close')
-      .then(file => file.close())
-      .then(
-        () => {
-          this.fds.delete(fd);
-          this.releasedFds.push(fd);
-          callback(null);
-        },
-        error => {
-          callback(error);
-        },
-      );
+    this.__close(fd)
+      .then(() => callback(null), error => callback(error));
   };
 
   public readonly read: FsCallbackApi['read'] = (
@@ -115,16 +105,27 @@ export class FsaNodeFs extends FsaNodeCore implements FsCallbackApi, FsSynchrono
       optHelpers.getReadFileOptions,
     )(a, b);
     const flagsNum = util.flagsToNumber(opts.flag);
-    return this.__getFileById(id, 'readFile')
-      .then(file => file.getFile())
-      .then(file => file.arrayBuffer())
-      .then(data => {
-        const buffer = Buffer.from(data);
-        callback(null, util.bufferToEncoding(buffer, opts.encoding));
-      })
-      .catch(error => {
-        callback(error);
-      });
+    (async () => {
+      let fd: number = typeof id === 'number' ? id : -1;
+      const originalFd = fd;
+      try {
+        if (fd === -1) {
+          const filename = util.pathToFilename(id as misc.PathLike);
+          fd = (await this.__open(filename, flagsNum, 0)).fd;
+        }
+        const handle = await this.__getFileById(fd, 'readFile');
+        const file = await handle.getFile();
+        const buffer = Buffer.from(await file.arrayBuffer());
+        return util.bufferToEncoding(buffer, opts.encoding)  
+      } finally {
+        try {
+          const idWasFd = typeof originalFd === 'number' && originalFd >= 0;
+          if (idWasFd) await this.__close(originalFd);
+        } catch {}
+      }
+    })()
+      .then(data => callback(null, data))
+      .catch(error => callback(error));
   };
 
   public readonly write: FsCallbackApi['write'] = (
@@ -198,11 +199,23 @@ export class FsaNodeFs extends FsaNodeCore implements FsCallbackApi, FsSynchrono
     const modeNum = util.modeToNumber(opts.mode);
     const buf = util.dataToBuffer(data, opts.encoding);
     (async () => {
-      const createIfMissing = !!(flagsNum & FLAG.O_CREAT);
-      const file = await this.__getFileById(id, 'writeFile', createIfMissing);
-      const writable = await file.createWritable({ keepExistingData: false });
-      await writable.write(buf);
-      await writable.close();
+      let fd: number = typeof id === 'number' ? id : -1;
+      const originalFd = fd;
+      try {
+        if (fd === -1) {
+          const filename = util.pathToFilename(id as misc.PathLike);
+          fd = (await this.__open(filename, flagsNum, modeNum)).fd;
+        }
+        const file = await this.__getFileById(fd, 'writeFile');
+        const writable = await file.createWritable({ keepExistingData: false });
+        await writable.write(buf);
+        await writable.close();
+      } finally {
+        try {
+          const idWasFd = typeof originalFd === 'number' && originalFd >= 0;
+          if (idWasFd) await this.__close(originalFd);
+        } catch {}
+      }
     })().then(
       () => cb(null),
       error => cb(error),

src/fsa-to-node/__tests__/FsaNodeFs.test.ts

@@ -5,6 +5,7 @@ import { IDirent, IStats } from '../../node/types/misc';
 import { FsaNodeFs } from '../FsaNodeFs';
 import { tick, until, of } from 'thingies';
 import { onlyOnNode20 } from '../../__tests__/util';
+import {FLAG} from '../../consts/FLAG';
 
 const setup = (json: NestedDirectoryJSON | null = null, mode: 'read' | 'readwrite' = 'readwrite') => {
   const mfs = memfs({ mountpoint: json }) as IFsWithVolume;
@@ -26,6 +27,17 @@ onlyOnNode20('FsaNodeFs', () => {
       expect(mfs.statSync('/mountpoint/test').isDirectory()).toBe(true);
     });
 
+    test('can create a sub-folder with trailing slash', async () => {
+      const { fs, mfs } = setup();
+      await new Promise<void>((resolve, reject) =>
+        fs.mkdir('/test/', err => {
+          if (err) return reject(err);
+          return resolve();
+        }),
+      );
+      expect(mfs.statSync('/mountpoint/test').isDirectory()).toBe(true);
+    });
+
     test('throws when creating sub-sub-folder', async () => {
       const { fs } = setup();
       try {
@@ -654,6 +666,20 @@ onlyOnNode20('FsaNodeFs', () => {
         '/mountpoint/f.html': 'test',
       });
     });
+
+    test('throws "EEXIST", if file already exists and O_EXCL flag set', async () => {
+      const { fs } = setup({ folder: { file: 'test' }, 'empty-folder': null, 'f.html': 'test' });
+      const [, err] = await of(fs.promises.writeFile('/folder/file', 'bar', { flag: 'wx' }));
+      expect(err).toBeInstanceOf(Error);
+      expect((<any>err).code).toBe('EEXIST');
+    });
+
+    test('throws "ENOENT", if file does not exist and O_CREAT flag not set', async () => {
+      const { fs } = setup({ folder: { file: 'test' }, 'empty-folder': null, 'f.html': 'test' });
+      const [, err] = await of(fs.promises.writeFile('/folder/file2', 'bar', { flag: FLAG.O_RDWR }));
+      expect(err).toBeInstanceOf(Error);
+      expect((<any>err).code).toBe('ENOENT');
+    });
   });
 
   describe('.read()', () => {

src/fsa-to-node/__tests__/util.test.ts

@@ -13,6 +13,10 @@ describe('pathToLocation()', () => {
     expect(pathToLocation('scary.exe')).toStrictEqual([[], 'scary.exe']);
   });
 
+  test('strips trailing slash', () => {
+    expect(pathToLocation('scary.exe/')).toStrictEqual([[], 'scary.exe']);
+  });
+
   test('multiple steps in the path', () => {
     expect(pathToLocation('/gg/wp/hf/gl.txt')).toStrictEqual([['gg', 'wp', 'hf'], 'gl.txt']);
     expect(pathToLocation('gg/wp/hf/gl.txt')).toStrictEqual([['gg', 'wp', 'hf'], 'gl.txt']);

src/fsa-to-node/util.ts

@@ -4,6 +4,7 @@ import type { FsLocation } from './types';
 
 export const pathToLocation = (path: string): FsLocation => {
   if (path[0] === FsaToNodeConstants.Separator) path = path.slice(1);
+  if (path[path.length - 1] === FsaToNodeConstants.Separator) path = path.slice(0, -1);
   const lastSlashIndex = path.lastIndexOf(FsaToNodeConstants.Separator);
   if (lastSlashIndex === -1) return [[], path];
   const file = path.slice(lastSlashIndex + 1);