Skip to content

Commit 99c8b67

Browse files
sirozhaclaude
andcommitted
fix: recover from stale-chunk and DOM-desync SPA crashes
Two crashes seen in production, both reproduced on the live app: - "Failed to fetch dynamically imported module": after a redeploy rotates the hashed chunk filenames, an open tab imports a deleted one. The server answered a missing /assets/* with 301 -> index.html (HTML for a JS module -> a MIME failure); it now returns 404 + no-store. The client listens for Vite's vite:preloadError and reloads once (debounced) to pull the current build. Hashed assets are served immutable; index.html and SPA routes no-cache. - "Failed to execute 'removeChild' ... not a child of this node": an external agent (a browser extension or auto-translation) mutates the DOM React owns, desyncing reconciliation. A root react-router errorElement catches this commit-phase crash and self-heals with a debounced reload, instead of React Router's dead default error screen. translate="no" opts the English-only UI out of the one trigger it can prevent (browser translation); the errorElement covers the rest regardless of source. Verified by reproducing both on the live old build (missing-chunk 301->HTML; extension/translation DOM mutation -> the exact removeChild crash) and confirming the fixed build recovers from each. Adds chunk-reload + RouteErrorBoundary unit tests and a static-serving integration test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9e1b093 commit 99c8b67

10 files changed

Lines changed: 502 additions & 25 deletions

File tree

backend/pkg/server/middleware.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package router
22

33
import (
4+
"net/http"
5+
"strings"
6+
47
"pentagi/pkg/server/models"
58
"pentagi/pkg/server/response"
69

@@ -34,3 +37,21 @@ func noCacheMiddleware() gin.HandlerFunc {
3437
c.Next()
3538
}
3639
}
40+
41+
// staticCacheMiddleware sets cache policy for the locally-served SPA build:
42+
// content-hashed /assets/* are immutable; everything else resolving to the SPA
43+
// (index.html, client routes) is no-cache, so a redeploy is picked up instead of
44+
// replaying a stale index.html that imports chunks the deploy already deleted.
45+
func staticCacheMiddleware() gin.HandlerFunc {
46+
return func(c *gin.Context) {
47+
if c.Request.Method == http.MethodGet && !strings.HasPrefix(c.Request.URL.Path, baseURL) {
48+
if strings.HasPrefix(c.Request.URL.Path, "/assets/") {
49+
c.Header("Cache-Control", "public, max-age=31536000, immutable")
50+
} else {
51+
c.Header("Cache-Control", "no-cache")
52+
}
53+
}
54+
55+
c.Next()
56+
}
57+
}

backend/pkg/server/router.go

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -337,36 +337,55 @@ func NewRouter(
337337
}
338338
}())
339339
} else {
340-
router.Use(static.Serve("/", static.LocalFile(cfg.StaticDir, true)))
340+
registerStaticFileServer(router, cfg.StaticDir)
341+
}
341342

342-
indexExists := true
343-
indexPath := filepath.Join(cfg.StaticDir, "index.html")
344-
if _, err := os.Stat(indexPath); err != nil {
345-
indexExists = false
346-
}
343+
return router
344+
}
347345

348-
router.NoRoute(func(c *gin.Context) {
349-
if c.Request.Method == "GET" && !strings.HasPrefix(c.Request.URL.Path, baseURL) {
350-
isFrontendRoute := false
351-
path := c.Request.URL.Path
352-
for _, prefix := range frontendRoutes {
353-
if path == prefix || strings.HasPrefix(path, prefix+"/") {
354-
isFrontendRoute = true
355-
break
356-
}
346+
// registerStaticFileServer serves the locally-built SPA (used when no STATIC_URL
347+
// upstream is set): cache headers, hashed assets via static.Serve, and an SPA
348+
// fallback that serves index.html for client routes but returns 404 for a missing
349+
// /assets/* — so the module loader fails cleanly and the app can reload to recover
350+
// instead of getting index.html and a MIME error. Split out to be unit-testable.
351+
func registerStaticFileServer(router *gin.Engine, staticDir string) {
352+
router.Use(staticCacheMiddleware())
353+
router.Use(static.Serve("/", static.LocalFile(staticDir, true)))
354+
355+
indexExists := true
356+
indexPath := filepath.Join(staticDir, "index.html")
357+
if _, err := os.Stat(indexPath); err != nil {
358+
indexExists = false
359+
}
360+
361+
router.NoRoute(func(c *gin.Context) {
362+
if c.Request.Method == http.MethodGet && !strings.HasPrefix(c.Request.URL.Path, baseURL) {
363+
isFrontendRoute := false
364+
path := c.Request.URL.Path
365+
for _, prefix := range frontendRoutes {
366+
if path == prefix || strings.HasPrefix(path, prefix+"/") {
367+
isFrontendRoute = true
368+
break
357369
}
370+
}
358371

359-
if isFrontendRoute && indexExists {
360-
c.File(indexPath)
361-
return
362-
}
372+
if isFrontendRoute && indexExists {
373+
c.File(indexPath)
374+
return
363375
}
364376

365-
c.Redirect(http.StatusMovedPermanently, "/")
366-
})
367-
}
377+
if strings.HasPrefix(path, "/assets/") {
378+
// A missing hashed asset may be a transient rolling-deploy
379+
// race, so override the immutable directive set above —
380+
// never cache this 404 as a permanent negative.
381+
c.Header("Cache-Control", "no-store")
382+
c.Status(http.StatusNotFound)
383+
return
384+
}
385+
}
368386

369-
return router
387+
c.Redirect(http.StatusMovedPermanently, "/")
388+
})
370389
}
371390

372391
func setKnowledgeGroup(parent *gin.RouterGroup, svc *services.KnowledgeService) {
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package router
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
10+
"github.com/gin-gonic/gin"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// newStaticTestServer builds a gin engine wired exactly like the production
16+
// local-serving branch (registerStaticFileServer) over a throwaway dist dir
17+
// holding one hashed asset and an index.html.
18+
func newStaticTestServer(t *testing.T) *gin.Engine {
19+
t.Helper()
20+
gin.SetMode(gin.TestMode)
21+
22+
dir := t.TempDir()
23+
require.NoError(t, os.WriteFile(filepath.Join(dir, "index.html"), []byte("<!doctype html><title>app</title>"), 0o600))
24+
require.NoError(t, os.Mkdir(filepath.Join(dir, "assets"), 0o750))
25+
require.NoError(t, os.WriteFile(filepath.Join(dir, "assets", "app-abc123.js"), []byte("export const x = 1;\n"), 0o600))
26+
27+
engine := gin.New()
28+
registerStaticFileServer(engine, dir)
29+
30+
return engine
31+
}
32+
33+
func getStatic(t *testing.T, engine *gin.Engine, path string) *httptest.ResponseRecorder {
34+
t.Helper()
35+
rec := httptest.NewRecorder()
36+
engine.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
37+
38+
return rec
39+
}
40+
41+
func TestStaticFileServer(t *testing.T) {
42+
engine := newStaticTestServer(t)
43+
44+
t.Run("existing hashed asset is served immutable", func(t *testing.T) {
45+
rec := getStatic(t, engine, "/assets/app-abc123.js")
46+
47+
assert.Equal(t, http.StatusOK, rec.Code)
48+
assert.Equal(t, "public, max-age=31536000, immutable", rec.Header().Get("Cache-Control"))
49+
})
50+
51+
t.Run("missing asset is 404 and never cached as a permanent negative", func(t *testing.T) {
52+
rec := getStatic(t, engine, "/assets/missing-deadbeef.js")
53+
54+
// 404 (not 301->HTML) so the browser module loader fails cleanly and
55+
// the SPA reloads; no-store (not immutable) so a transient rolling
56+
// deploy isn't cached as a permanent miss.
57+
assert.Equal(t, http.StatusNotFound, rec.Code)
58+
assert.Equal(t, "no-store", rec.Header().Get("Cache-Control"))
59+
assert.NotContains(t, rec.Header().Get("Cache-Control"), "immutable")
60+
})
61+
62+
t.Run("index.html is served revalidated", func(t *testing.T) {
63+
rec := getStatic(t, engine, "/")
64+
65+
assert.Equal(t, http.StatusOK, rec.Code)
66+
assert.Equal(t, "no-cache", rec.Header().Get("Cache-Control"))
67+
})
68+
69+
t.Run("SPA deep-link falls back to index.html with no-cache", func(t *testing.T) {
70+
rec := getStatic(t, engine, "/templates")
71+
72+
assert.Equal(t, http.StatusOK, rec.Code)
73+
assert.Equal(t, "no-cache", rec.Header().Get("Cache-Control"))
74+
assert.Contains(t, rec.Body.String(), "<!doctype html>")
75+
})
76+
77+
t.Run("unknown non-asset path redirects to root", func(t *testing.T) {
78+
rec := getStatic(t, engine, "/favicon-not-there.ico")
79+
80+
assert.Equal(t, http.StatusMovedPermanently, rec.Code)
81+
assert.Equal(t, "/", rec.Header().Get("Location"))
82+
})
83+
84+
t.Run("api paths are untouched by the static cache policy", func(t *testing.T) {
85+
rec := getStatic(t, engine, baseURL+"/anything")
86+
87+
assert.Empty(t, rec.Header().Get("Cache-Control"))
88+
})
89+
}

frontend/index.html

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
<!doctype html>
2-
<html lang="en">
2+
<html
3+
lang="en"
4+
translate="no"
5+
>
36
<head>
47
<meta charset="UTF-8" />
8+
<!-- The UI is English-only. Browser auto-translation is one known trigger of
9+
a DOM desync (it swaps text nodes React owns), so opt the document out of
10+
it. Defense-in-depth only: the route errorElement recovers from such
11+
desyncs (extension or translation) regardless of source. -->
12+
<meta
13+
name="google"
14+
content="notranslate"
15+
/>
516
<meta
617
name="viewport"
718
content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no"

frontend/src/app.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import ProtectedRoute from '@/components/routes/protected-route';
1717
import PublicRoute from '@/components/routes/public-route';
1818
import { DocumentTitle } from '@/components/shared/document-title';
1919
import PageLoader from '@/components/shared/page-loader';
20+
import RouteErrorBoundary from '@/components/shared/route-error-boundary';
2021
import { Toaster } from '@/components/ui/sonner';
2122
import client from '@/lib/apollo';
2223
import { routeTitles } from '@/lib/route-titles';
@@ -126,7 +127,10 @@ function RootLayout() {
126127

127128
const router = createBrowserRouter(
128129
createRoutesFromElements(
129-
<Route element={<RootLayout />}>
130+
<Route
131+
element={<RootLayout />}
132+
errorElement={<RouteErrorBoundary />}
133+
>
130134
{/* private routes */}
131135
<Route element={<ProtectedAppLayout />}>
132136
{/* Main layout for chat pages */}
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { render, screen, waitFor } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import { createMemoryRouter, RouterProvider } from 'react-router-dom';
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
5+
6+
import RouteErrorBoundary from './route-error-boundary';
7+
8+
// A loader that throws routes react-router to our `errorElement`, so
9+
// `useRouteError` inside the boundary receives exactly this value — the same
10+
// path a real chunk-load or render failure takes.
11+
const renderWithRouteError = (error: unknown) => {
12+
const router = createMemoryRouter(
13+
[
14+
{
15+
element: <div>page</div>,
16+
errorElement: <RouteErrorBoundary />,
17+
loader: () => {
18+
throw error;
19+
},
20+
path: '/',
21+
},
22+
],
23+
{ initialEntries: ['/'] },
24+
);
25+
26+
return render(<RouterProvider router={router} />);
27+
};
28+
29+
const chunkError = () =>
30+
new Error('Failed to fetch dynamically imported module: https://app/assets/templates-D01Ouz7C.js');
31+
const renderError = () => new TypeError('Cannot read properties of undefined (reading length)');
32+
const domDesyncError = () =>
33+
new DOMException(
34+
"Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.",
35+
'NotFoundError',
36+
);
37+
38+
describe('RouteErrorBoundary', () => {
39+
const originalLocation = window.location;
40+
let reloadSpy: ReturnType<typeof vi.fn>;
41+
42+
beforeEach(() => {
43+
sessionStorage.clear();
44+
reloadSpy = vi.fn();
45+
// jsdom's `location.reload` is non-configurable, so swap the whole
46+
// `location` object — the same workaround chunk-reload.test.ts uses.
47+
Object.defineProperty(window, 'location', {
48+
configurable: true,
49+
value: { ...originalLocation, reload: reloadSpy },
50+
writable: true,
51+
});
52+
// react-router logs loader errors it routes to the boundary; keep the
53+
// test output clean without hiding real assertion failures.
54+
vi.spyOn(console, 'error').mockImplementation(() => {});
55+
});
56+
57+
afterEach(() => {
58+
vi.restoreAllMocks();
59+
Object.defineProperty(window, 'location', {
60+
configurable: true,
61+
value: originalLocation,
62+
writable: true,
63+
});
64+
sessionStorage.clear();
65+
});
66+
67+
it('shows the redeploy message and auto-reloads once for a chunk-load error', async () => {
68+
renderWithRouteError(chunkError());
69+
70+
expect(await screen.findByText(/a new version was likely just deployed/i)).toBeInTheDocument();
71+
expect(screen.getByRole('alert')).toBeInTheDocument();
72+
await waitFor(() => expect(reloadSpy).toHaveBeenCalledTimes(1));
73+
});
74+
75+
it('shows the glitch message and auto-reloads once for a DOM-desync (removeChild) error', async () => {
76+
renderWithRouteError(domDesyncError());
77+
78+
expect(await screen.findByText(/display glitch/i)).toBeInTheDocument();
79+
await waitFor(() => expect(reloadSpy).toHaveBeenCalledTimes(1));
80+
});
81+
82+
it('shows the generic message and does not auto-reload for a non-chunk error', async () => {
83+
renderWithRouteError(renderError());
84+
85+
expect(await screen.findByText(/ran into an unexpected error/i)).toBeInTheDocument();
86+
expect(reloadSpy).not.toHaveBeenCalled();
87+
});
88+
89+
it('reloads when the user clicks Reload', async () => {
90+
renderWithRouteError(renderError());
91+
92+
await userEvent.click(await screen.findByRole('button', { name: /reload/i }));
93+
94+
expect(reloadSpy).toHaveBeenCalledTimes(1);
95+
});
96+
});

0 commit comments

Comments
 (0)