Merge pull request #259 from effigies/fix/file-access-errors

feat: Add file access functions that translate errors into issues

Author: rwblair

changelog.d/20250911_171112_markiewicz_file_access_errors.md

@@ -0,0 +1,47 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+For top level release notes, leave all the headers commented out.
+-->
+
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+### Fixed
+
+- File access failures consistently produce `FILE_READ` errors across all file types.
+
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->
+<!--
+### Infrastructure
+
+- A bullet item for the Infrastructure category.
+
+-->

src/files/access.test.ts

@@ -0,0 +1,40 @@
+import { assert, assertArrayIncludes, assertObjectMatch } from '@std/assert'
+import { basename, dirname } from '@std/path'
+import { BIDSFileDeno } from './deno.ts'
+
+export function testAsyncFileAccess(
+  name: string,
+  fn: (file: BIDSFileDeno, ...args: any[]) => Promise<any>,
+  ...args: any[]
+) {
+  Deno.test({
+    name,
+    ignore: Deno.build.os === 'windows',
+    async fn(t) {
+      await t.step('Dangling symlink', async () => {
+        const file = new BIDSFileDeno('tests/data', '/broken-symlink')
+        try {
+          await fn(file, ...args)
+          assert(false, 'Expected error')
+        } catch (e: any) {
+          assertObjectMatch(e, {
+            code: 'FILE_READ',
+            location: '/broken-symlink',
+          })
+          assertArrayIncludes(['NotFound', 'FilesystemLoop'], [e.subCode])
+        }
+      })
+      await t.step('Insufficient permissions', async () => {
+        const tmpfile = await Deno.makeTempFile()
+        await Deno.chmod(tmpfile, 0o000)
+        const file = new BIDSFileDeno('', tmpfile)
+        try {
+          await fn(file, ...args)
+          assert(false, 'Expected error')
+        } catch (e: any) {
+          assertObjectMatch(e, { code: 'FILE_READ', subCode: 'PermissionDenied' })
+        }
+      })
+    },
+  })
+}

src/files/access.ts

@@ -0,0 +1,35 @@
+import { type BIDSFile } from '../types/filetree.ts'
+import { type Issue } from '../types/issues.ts'
+
+function IOErrorToIssue(err: { code: string; name: string }): Issue {
+  const subcode = err.name
+  let issueMessage: string | undefined = undefined
+  if (err.code === 'ENOENT' || err.code === 'ELOOP') {
+    issueMessage = 'Possible dangling symbolic link'
+  }
+  return { code: 'FILE_READ', subCode: err.name, issueMessage }
+}
+
+export function openStream(file: BIDSFile): ReadableStream<Uint8Array<ArrayBuffer>> {
+  try {
+    return file.stream
+  } catch (err: any) {
+    throw { location: file.path, ...IOErrorToIssue(err) }
+  }
+}
+
+export async function readBytes(
+  file: BIDSFile,
+  size: number,
+  offset = 0,
+): Promise<Uint8Array<ArrayBuffer>> {
+  return file.readBytes(size, offset).catch((err: any) => {
+    throw { location: file.path, ...IOErrorToIssue(err) }
+  })
+}
+
+export async function readText(file: BIDSFile): Promise<string> {
+  return file.text().catch((err: any) => {
+    throw { location: file.path, ...IOErrorToIssue(err) }
+  })
+}

src/files/gzip.test.ts

@@ -1,6 +1,7 @@
 import { assert, assertObjectMatch } from '@std/assert'
 import { parseGzip } from './gzip.ts'
 import { BIDSFileDeno } from './deno.ts'
+import { testAsyncFileAccess } from './access.test.ts'
 
 Deno.test('parseGzip', async (t) => {
   await t.step('parses anonymized file', async () => {
@@ -40,3 +41,5 @@ Deno.test('parseGzip', async (t) => {
     assert(!gzip)
   })
 })
+
+testAsyncFileAccess('Test file access errors for parseGzip', parseGzip)

src/files/gzip.ts

@@ -4,6 +4,7 @@
  */
 import type { Gzip } from '@bids/schema/context'
 import type { BIDSFile } from '../types/filetree.ts'
+import { readBytes } from './access.ts'
 
 /**
  * Parse a gzip header from a file
@@ -19,7 +20,7 @@ export async function parseGzip(
   file: BIDSFile,
   maxBytes: number = 512,
 ): Promise<Gzip | undefined> {
-  const buf = await file.readBytes(maxBytes)
+  const buf = await readBytes(file, maxBytes)
   const view = new DataView(buf.buffer, buf.byteOffset, buf.byteLength)
   if (view.byteLength < 2 || view.getUint16(0, false) !== 0x1f8b) return undefined
 

src/files/json.test.ts

@@ -1,6 +1,7 @@
 import { type assert, assertObjectMatch } from '@std/assert'
 import type { BIDSFile } from '../types/filetree.ts'
 import type { FileIgnoreRules } from './ignore.ts'
+import { testAsyncFileAccess } from './access.test.ts'
 
 import { loadJSON } from './json.ts'
 
@@ -61,3 +62,5 @@ Deno.test('Test JSON error conditions', async (t) => {
     assertObjectMatch(error, { code: 'JSON_INVALID' })
   })
 })
+
+testAsyncFileAccess('Test file access errors for loadJSON', loadJSON)

src/files/json.ts

@@ -1,12 +1,13 @@
 import type { BIDSFile } from '../types/filetree.ts'
+import { readBytes } from './access.ts'
 
 async function readJSONText(file: BIDSFile): Promise<string> {
   // Read JSON text from a file
   // JSON must be encoded in UTF-8 without a byte order mark (BOM)
   const decoder = new TextDecoder('utf-8', { fatal: true, ignoreBOM: true })
   // Streaming TextDecoders are buggy in Deno and Chrome, so read the
   // entire file into memory before decoding and parsing
-  const data = await file.readBytes(file.size)
+  const data = await readBytes(file, file.size)
   try {
     const text = decoder.decode(data)
     if (text.startsWith('\uFEFF')) {

src/files/nifti.test.ts

@@ -1,6 +1,7 @@
 import { assert, assertEquals, assertObjectMatch } from '@std/assert'
 import { FileIgnoreRules } from './ignore.ts'
 import { BIDSFileDeno } from './deno.ts'
+import { testAsyncFileAccess } from './access.test.ts'
 
 import { axisCodes, loadHeader } from './nifti.ts'
 
@@ -96,3 +97,5 @@ Deno.test('Test extracting axis codes', async (t) => {
     assertEquals(axisCodes(affine), ['A', 'S', 'R'])
   })
 })
+
+testAsyncFileAccess('Test file access errors for loadHeader', loadHeader)

src/files/nifti.ts

@@ -2,6 +2,7 @@ import { isCompressed, isNIFTI1, isNIFTI2, NIFTI1, NIFTI2 } from '@mango/nifti'
 import type { BIDSFile } from '../types/filetree.ts'
 import { logger } from '../utils/logger.ts'
 import type { NiftiHeader } from '@bids/schema/context'
+import { readBytes } from './access.ts'
 
 async function extract(buffer: Uint8Array, nbytes: number): Promise<Uint8Array<ArrayBuffer>> {
   // The fflate decompression that is used in nifti-reader does not like
@@ -32,8 +33,8 @@ async function extract(buffer: Uint8Array, nbytes: number): Promise<Uint8Array<A
 }
 
 export async function loadHeader(file: BIDSFile): Promise<NiftiHeader> {
+  const buf = await readBytes(file, 1024)
   try {
-    const buf = await file.readBytes(1024)
     const data = isCompressed(buf.buffer) ? await extract(buf, 540) : buf.slice(0, 540)
     let header
     if (isNIFTI1(data.buffer)) {

src/files/tiff.test.ts

@@ -1,6 +1,7 @@
 import { assert, assertObjectMatch } from '@std/assert'
 import { parseTIFF } from './tiff.ts'
 import { BIDSFileDeno } from './deno.ts'
+import { testAsyncFileAccess } from './access.test.ts'
 
 Deno.test('parseTIFF', async (t) => {
   await t.step('parse example file as TIFF', async () => {
@@ -53,3 +54,5 @@ Deno.test('parseTIFF', async (t) => {
     })
   })
 })
+
+testAsyncFileAccess('Test file access errors for parseTIFF', parseTIFF)