fix(fs): report read errors of readTextFileLines (#3625)

* fix(fs): report read errors of readTextFileLines

In plugins/fs/src/commands.rs read_text_file_lines_next, a read error was
mapped to "not done, empty line", so a persistent error such as EISDIR
(File::open of a directory succeeds on Unix) made the iterator yield ""
forever and the error was swallowed.

The command now closes the resource and returns the error; the JS iterator
resets its rid so a new iteration starts over. api-iife.js regenerated. E2E
spec iterates a directory and expects a rejection.

* fix(fs): close the file when a readTextFileLines loop exits early

In plugins/fs/guest-js/index.ts readTextFileLines, the async iterator did
not implement return(), so breaking out of a for await loop left the
StdLinesResource (an open file) in the webview resource table.

return() now closes the resource and resets the iterator. api-iife.js
regenerated. E2E spec breaks out of a loop, checks the resource id is no
longer valid and that iterating again starts over.

.changes/fix-fs-read-lines-error.md

@@ -0,0 +1,6 @@
+---
+fs: patch
+fs-js: patch
+---
+
+Fixed `readTextFileLines` yielding empty lines forever when reading fails (e.g. on a directory). The read error is now reported: the iterator rejects and the file is closed.

.changes/fix-fs-read-lines-return.md

@@ -0,0 +1,6 @@
+---
+fs: patch
+fs-js: patch
+---
+
+Fixed `readTextFileLines` leaving the file open until the webview is destroyed when a `for await` loop over it exits early (`break`, `return` or `throw`). The iterator now implements `return()`, which closes the file.

packages/api-e2e/test/specs/fs.spec.ts

@@ -234,6 +234,57 @@ describePlugin('fs', () => {
     expect(lines).toEqual(['one', 'two', 'three'])
   })
 
+  it('readTextFileLines closes the file when a loop exits early', async () => {
+    const result = await tauri(async (api, path) => {
+      const baseDir = api.fs.BaseDirectory.AppData
+      await api.fs.writeTextFile(path, 'one\ntwo\nthree', { baseDir })
+      const lines = await api.fs.readTextFileLines(path, { baseDir })
+      // the iterator keeps the id of the open file in `rid`
+      const state = lines as unknown as { rid: number | null }
+      let rid: number | null = null
+      let first: string | null = null
+      for await (const line of lines) {
+        first = line
+        rid = state.rid
+        break
+      }
+      let closeError: string | null = null
+      try {
+        await api.core.invoke('plugin:resources|close', { rid })
+      } catch (error) {
+        closeError = String(error)
+      }
+      // iterating again starts over
+      const all: string[] = []
+      for await (const line of lines) {
+        all.push(line)
+      }
+      return { first, ridAfter: state.rid, closeError, all }
+    }, `${dir}/lines-break.txt`)
+    expect(result.first).toBe('one')
+    expect(result.ridAfter).toBeNull()
+    // the resource was already closed by the iterator
+    expect(result.closeError).toMatch(/resource id \d+ is invalid/)
+    expect(result.all).toEqual(['one', 'two', 'three'])
+  })
+
+  it('readTextFileLines rejects when the file cannot be read', async () => {
+    // a directory: opening it fails on Windows, reading it fails elsewhere,
+    // which used to yield empty lines forever
+    const message = await tauriError(async (api, path) => {
+      const baseDir = api.fs.BaseDirectory.AppData
+      await api.fs.mkdir(path, { baseDir, recursive: true })
+      const lines = await api.fs.readTextFileLines(path, { baseDir })
+      for (let i = 0; i < 1000; i++) {
+        const { done } = await lines.next()
+        if (done) return
+      }
+      throw new Error('readTextFileLines kept yielding lines for a directory')
+    }, `${dir}/lines-dir`)
+    expect(message).not.toMatch(/kept yielding/)
+    expect(message).toMatch(/failed to (read line|open file)/)
+  })
+
   it('FileHandle supports write, seek, read, stat and truncate', async () => {
     const result = await tauri(async (api, path) => {
       const baseDir = api.fs.BaseDirectory.AppData

plugins/fs/guest-js/index.ts

@@ -887,10 +887,17 @@ async function readTextFileLines(
         })
       }
 
-      const arr = await invoke<ArrayBuffer | number[]>(
-        'plugin:fs|read_text_file_lines_next',
-        { rid: this.rid }
-      )
+      let arr: ArrayBuffer | number[]
+      try {
+        arr = await invoke<ArrayBuffer | number[]>(
+          'plugin:fs|read_text_file_lines_next',
+          { rid: this.rid }
+        )
+      } catch (error) {
+        // the resource is closed on errors, the next iteration starts over
+        this.rid = null
+        throw error
+      }
 
       const bytes =
         arr instanceof ArrayBuffer ? new Uint8Array(arr) : Uint8Array.from(arr)
@@ -916,6 +923,17 @@ async function readTextFileLines(
       }
     },
 
+    // called when a `for await` loop exits early (`break`, `return` or `throw`)
+    async return(): Promise<IteratorResult<string>> {
+      if (this.rid !== null) {
+        const rid = this.rid
+        this.rid = null
+        // close the file, otherwise it stays open until the webview is destroyed
+        await new Resource(rid).close()
+      }
+      return { value: null, done: true }
+    },
+
     [Symbol.asyncIterator](): AsyncIterableIterator<string> {
       return this
     }

plugins/fs/src/commands.rs

@@ -703,7 +703,12 @@ pub async fn read_text_file_lines_next<R: Runtime>(
                 bytes.push(false as u8);
                 Ok(bytes)
             }
-            Some(Err(_)) => Ok(vec![false as u8]),
+            Some(Err(e)) => {
+                // the error may be persistent (e.g. reading a directory), do not report
+                // an empty line and let the caller loop forever
+                resource_table.close(rid)?;
+                Err(format!("failed to read line with error: {e}").into())
+            }
             None => {
                 resource_table.close(rid)?;
                 Ok(vec![true as u8])