Fix preview crash on re-render (BadResource error) #13967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open

cderv wants to merge 10 commits into main from fix/issue-13955

+205 −10

news/changelog-1.9.md

-Original file line number
+Diff line change
@@ Expand Up / @@ -125,6 +125,10 @@ All changes included in 1.9: @@
     - ([#4426](https://github.com/quarto-dev/quarto-cli/issues/4426)): New `quarto install verapdf` command installs [veraPDF](https://verapdf.org/) for PDF/A and PDF/UA validation. When verapdf is available, PDFs created with the `pdf-standard` option are automatically validated for compliance. Also supports `quarto uninstall verapdf`, `quarto update verapdf`, and `quarto tools`.
+    ### `preview`
+    - ([#13804](https://github.com/quarto-dev/quarto-cli/pull/13804)): Fix intermittent preview crashes during re-renders by properly managing project context lifecycle. Resolves issues with missing temporary directories and `quarto_ipynb` files when editing notebooks and qmd files together.
     ## Extensions
     - Metadata and brand extensions now work without a `_quarto.yml` project. (Engine extensions do too.) A temporary default project is created in memory.
@@ Expand Down @@

src/command/preview/preview.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -4,7 +4,7 @@ @@
      * Copyright (C) 2020-2022 Posit Software, PBC
      */
-    import { info, warning } from "../../deno_ral/log.ts";
+    import { debug, info, warning } from "../../deno_ral/log.ts";
     import {
       basename,
       dirname,
@@ Expand Down Expand Up / @@ -424,6 +424,14 @@ export async function renderForPreview( @@
       pandocArgs: string[],
       project?: ProjectContext,
     ): Promise<RenderForPreviewResult> {
+      // Invalidate file cache for the file being rendered so changes are picked up.
+      // The project context persists across re-renders in preview mode, but the
+      // fileInformationCache contains file content that needs to be refreshed.
+      // TODO(#13955): Consider adding a dedicated invalidateForFile() method on ProjectContext
+      if (project?.fileInformationCache) {
+        project.fileInformationCache.delete(file);
+      }
       // render
       const renderResult = await render(file, {
         services,
@@ Expand Down Expand Up / @@ -485,8 +493,6 @@ export async function renderForPreview( @@
         [],
       ));
-      renderResult.context.cleanup();
       return {
         file,
         format: renderResult.files[0].format,
@@ Expand Down @@

src/core/sass/cache.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -145,7 +145,9 @@ class SassCache implements Cloneable<SassCache> { @@
       // add a cleanup method to register a cleanup handler
       cleanup(temp: TempContext | undefined) {
         const registerCleanup = temp ? temp.onCleanup : onCleanup;
+        const cachePath = this.path;
         registerCleanup(() => {
+          log.debug(`SassCache cleanup executing for ${cachePath}`);
           try {
             this.kv.close();
             if (temp) safeRemoveIfExists(this.path);
@@ Expand Down @@

src/inspect/inspect.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -139,11 +139,13 @@ const populateFileInformation = async ( @@
       }
       await projectResolveCodeCellsForFile(context, engine, file);
       await projectFileMetadata(context, file);
-      fileInformation[file] = {
-        includeMap: context.fileInformationCache.get(file)?.includeMap ??
-          [],
-        codeCells: context.fileInformationCache.get(file)?.codeCells ?? [],
-        metadata: context.fileInformationCache.get(file)?.metadata ?? {},
+      const cacheEntry = context.fileInformationCache.get(file);
+      // Output key: project-relative for portability
+      const outputKey = relative(context.dir, normalizePath(file));
+      fileInformation[outputKey] = {
+        includeMap: cacheEntry?.includeMap ?? [],
+        codeCells: cacheEntry?.codeCells ?? [],
+        metadata: cacheEntry?.metadata ?? {},
       };
     };
@@ Expand Down @@

src/project/project-shared.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -506,7 +506,7 @@ export const ensureFileInformationCache = ( @@
       file: string,
     ) => {
       if (!project.fileInformationCache) {
-        project.fileInformationCache = new Map();
+        project.fileInformationCache = new FileInformationCacheMap();
       }
       assert(
         project.fileInformationCache instanceof Map,
@@ Expand Down Expand Up / @@ -656,8 +656,25 @@ export async function projectResolveBrand( @@
     }
     // Create a class that extends Map and implements Cloneable
+    // All operations normalize keys for cross-platform consistency
     export class FileInformationCacheMap extends Map<string, FileInformation>
       implements Cloneable<Map<string, FileInformation>> {
+      override get(key: string): FileInformation | undefined {
+        return super.get(normalizePath(key));
+      }
+      override has(key: string): boolean {
+        return super.has(normalizePath(key));
+      }
+      override set(key: string, value: FileInformation): this {
+        return super.set(normalizePath(key), value);
+      }
+      override delete(key: string): boolean {
+        return super.delete(normalizePath(key));
+      }
       clone(): Map<string, FileInformation> {
         // Return the same instance (reference) instead of creating a clone
         return this;
@@ Expand Down @@

src/project/types/single-file/single-file.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -45,10 +45,12 @@ export async function singleFileProjectContext( @@
       const temp = globalTempContext();
       const projectCacheBaseDir = temp.createDir();
+      const normalizedDir = normalizePath(dirname(source));
       const result: ProjectContext = {
         clone: () => result,
         resolveBrand: (fileName?: string) => projectResolveBrand(result, fileName),
-        dir: normalizePath(dirname(source)),
+        dir: normalizedDir,
         engines: [],
         files: {
           input: [],
@@ Expand Down @@

tests/unit/project/file-information-cache.test.ts

-Original file line number
+Diff line change
@@ -0,0 +1,109 @@
+    /*
+     * file-information-cache.test.ts
+     *
+     * Tests for fileInformationCache path normalization
+     * Related to issue #13955
+     *
+     * Copyright (C) 2020-2022 Posit Software, PBC
+     */
+    import { unitTest } from "../../test.ts";
+    import { assert } from "testing/asserts";
+    import { join } from "../../../src/deno_ral/path.ts";
+    import {
+      ensureFileInformationCache,
+      FileInformationCacheMap,
+    } from "../../../src/project/project-shared.ts";
+    import { createMockProjectContext } from "./utils.ts";
+    // deno-lint-ignore require-await
+    unitTest(
+      "fileInformationCache - same path returns same entry",
+      async () => {
+        const project = createMockProjectContext();
+        // Use cross-platform absolute path (backslashes on Windows, forward on Linux)
+        const path1 = join(project.dir, "doc.qmd");
+        const path2 = join(project.dir, "doc.qmd");
+        const entry1 = ensureFileInformationCache(project, path1);
+        const entry2 = ensureFileInformationCache(project, path2);
+        assert(
+          entry1 === entry2,
+          "Same path should return same cache entry",
+        );
+        assert(
+          project.fileInformationCache.size === 1,
+          "Should have exactly one cache entry",
+        );
+      },
+    );
+    // deno-lint-ignore require-await
+    unitTest(
+      "fileInformationCache - different paths create different entries",
+      async () => {
+        const project = createMockProjectContext();
+        const path1 = join(project.dir, "doc1.qmd");
+        const path2 = join(project.dir, "doc2.qmd");
+        const entry1 = ensureFileInformationCache(project, path1);
+        const entry2 = ensureFileInformationCache(project, path2);
+        assert(
+          entry1 !== entry2,
+          "Different paths should return different cache entries",
+        );
+        assert(
+          project.fileInformationCache.size === 2,
+          "Should have two cache entries for different paths",
+        );
+      },
+    );
+    // deno-lint-ignore require-await
+    unitTest(
+      "fileInformationCache - cache entry persists across calls",
+      async () => {
+        const project = createMockProjectContext();
+        const path = join(project.dir, "doc.qmd");
+        // First call creates entry
+        const entry1 = ensureFileInformationCache(project, path);
+        // Modify the entry
+        entry1.metadata = { title: "Test" };
+        // Second call should return same entry with our modification
+        const entry2 = ensureFileInformationCache(project, path);
+        assert(
+          entry2.metadata?.title === "Test",
+          "Cache entry should persist modifications",
+        );
+        assert(
+          entry1 === entry2,
+          "Should return same cache entry object",
+        );
+      },
+    );
+    // deno-lint-ignore require-await
+    unitTest(
+      "ensureFileInformationCache - creates FileInformationCacheMap when cache is missing",
+      async () => {
+        const project = createMockProjectContext();
+        // Simulate minimal ProjectContext without cache (as in command-utils.ts)
+        // deno-lint-ignore no-explicit-any
+        (project as any).fileInformationCache = undefined;
+        ensureFileInformationCache(project, join(project.dir, "doc.qmd"));
+        assert(
+          project.fileInformationCache instanceof FileInformationCacheMap,
+          "Should create FileInformationCacheMap, not plain Map",
+        );
+      },
+    );

tests/unit/project/utils.ts

-Original file line number
+Diff line change
@@ -0,0 +1,53 @@
+    /*
+     * utils.ts
+     *
+     * Test utilities for project-related unit tests
+     *
+     * Copyright (C) 2020-2022 Posit Software, PBC
+     */
+    import { ProjectContext } from "../../../src/project/types.ts";
+    import { FileInformationCacheMap } from "../../../src/project/project-shared.ts";
+    /**
+     * Create a minimal mock ProjectContext for testing.
+     * Only provides the essential properties needed for cache-related tests.
+     *
+     * @param dir - The project directory (defaults to a temp directory)
+     * @returns A mock ProjectContext suitable for unit testing
+     */
+    export function createMockProjectContext(
+      dir?: string,
+    ): ProjectContext {
+      const projectDir = dir ?? Deno.makeTempDirSync({ prefix: "quarto-test" });
+      const ownsDir = dir === undefined;
+      return {
+        dir: projectDir,
+        engines: [],
+        files: { input: [] },
+        notebookContext: {} as ProjectContext["notebookContext"],
+        fileInformationCache: new FileInformationCacheMap(),
+        resolveBrand: () => Promise.resolve(undefined),
+        resolveFullMarkdownForFile: () => Promise.resolve({} as never),
+        fileExecutionEngineAndTarget: () => Promise.resolve({} as never),
+        fileMetadata: () => Promise.resolve({}),
+        environment: () => Promise.resolve({} as never),
+        renderFormats: () => Promise.resolve({}),
+        clone: function () {
+          return this;
+        },
+        isSingleFile: false,
+        diskCache: {} as ProjectContext["diskCache"],
+        temp: {} as ProjectContext["temp"],
+        cleanup: () => {
+          if (ownsDir) {
+            try {
+              Deno.removeSync(projectDir, { recursive: true });
+            } catch {
+              // Ignore cleanup errors in tests
+            }
+          }
+        },
+      } as ProjectContext;
+    }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix preview crash on re-render (BadResource error) #13967

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Fix preview crash on re-render (BadResource error) #13967

Are you sure you want to change the base?

Fix preview crash on re-render (BadResource error) #13967

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!