diff --git a/internal/router/ui.go b/internal/router/ui.go index 14f71f5fc..0094b78b9 100644 --- a/internal/router/ui.go +++ b/internal/router/ui.go @@ -69,6 +69,19 @@ func (r *_resource) Open(name string) (fs.File, error) { return r.fs.Open(name) } +// isMissingStaticAsset reports whether the request targets a file under /static that does not exist. +// gin falls through to NoRoute for such paths, and answering with index.html (200) makes the browser +// parse HTML as JavaScript after an upgrade, when a stale tab requests chunks of the previous build. +func isMissingStaticAsset(urlPath, baseURLPath string) bool { + if len(baseURLPath) > 0 { + if !strings.HasPrefix(urlPath, baseURLPath) { + return false + } + urlPath = strings.TrimPrefix(urlPath, baseURLPath) + } + return strings.HasPrefix(urlPath, "/static/") +} + // Register a new static resource which generated by ui directory func (a *UIRouter) Register(r *gin.Engine, baseURLPath string) { staticPath := os.Getenv("ANSWER_STATIC_PATH") @@ -85,6 +98,10 @@ func (a *UIRouter) Register(r *gin.Engine, baseURLPath string) { r.LoadHTMLGlob(staticPath + "/*.html") r.Static(baseURLPath+"/static", staticPath+"/static") r.NoRoute(func(c *gin.Context) { + if isMissingStaticAsset(c.Request.URL.Path, baseURLPath) { + c.AbortWithStatus(http.StatusNotFound) + return + } c.HTML(http.StatusOK, "index.html", gin.H{}) }) @@ -100,6 +117,10 @@ func (a *UIRouter) Register(r *gin.Engine, baseURLPath string) { // specify the not router for default routes and redirect r.NoRoute(func(c *gin.Context) { + if isMissingStaticAsset(c.Request.URL.Path, baseURLPath) { + c.AbortWithStatus(http.StatusNotFound) + return + } urlPath := c.Request.URL.Path if len(baseURLPath) > 0 { urlPath = strings.TrimPrefix(urlPath, baseURLPath) diff --git a/internal/router/ui_test.go b/internal/router/ui_test.go index 645529338..e380699f4 100644 --- a/internal/router/ui_test.go +++ b/internal/router/ui_test.go @@ -19,41 +19,25 @@ package router -import ( - "errors" - "net/http" - "net/http/httptest" - "testing" - - "github.com/apache/answer/internal/service/mock" - "github.com/gin-gonic/gin" - "github.com/stretchr/testify/assert" - "go.uber.org/mock/gomock" -) - -func TestUIRouter_FaviconWithNilBranding(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockSiteInfoService := mock.NewMockSiteInfoCommonService(ctrl) - - // Simulate a database error - mockSiteInfoService.EXPECT(). - GetSiteBranding(gomock.Any()). - Return(nil, errors.New("database connection failed")) - - router := &UIRouter{ - siteInfoService: mockSiteInfoService, +import "testing" + +func TestIsMissingStaticAsset(t *testing.T) { + cases := []struct { + name, urlPath, base string + want bool + }{ + {"chunk of a previous build", "/static/js/6487.02e56745.chunk.js", "", true}, + {"with base url", "/answer/static/css/main.css", "/answer", true}, + {"spa route", "/users/alice", "", false}, + {"spa route with base url", "/answer/questions", "/answer", false}, + {"static under a different base", "/static/js/main.js", "/answer", false}, + {"root", "/", "", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := isMissingStaticAsset(c.urlPath, c.base); got != c.want { + t.Fatalf("isMissingStaticAsset(%q, %q) = %v, want %v", c.urlPath, c.base, got, c.want) + } + }) } - - gin.SetMode(gin.TestMode) - r := gin.New() - router.Register(r, "") - - req := httptest.NewRequest(http.MethodGet, "/favicon.ico", nil) - w := httptest.NewRecorder() - - r.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) } diff --git a/ui/src/router/index.tsx b/ui/src/router/index.tsx index ea2fdf3c2..2a6db31a4 100644 --- a/ui/src/router/index.tsx +++ b/ui/src/router/index.tsx @@ -27,6 +27,8 @@ import baseRoutes, { RouteNode } from './routes'; import RouteGuard from './RouteGuard'; import RouteErrorBoundary from './RouteErrorBoundary'; +const STALE_CHUNK_KEY = 'answer_stale_chunk_reload'; + const routeWrapper = (routeNodes: RouteNode[], root: RouteNode[]) => { routeNodes.forEach((rn) => { if (rn.page === 'pages/Layout') { @@ -48,7 +50,25 @@ const routeWrapper = (routeNodes: RouteNode[], root: RouteNode[]) => { if (typeof rn.page === 'string') { const pagePath = rn.page.replace('pages/', ''); - Ctrl = lazy(() => import(`@/pages/${pagePath}`)); + Ctrl = lazy(() => + import(`@/pages/${pagePath}`).then( + (m) => { + sessionStorage.removeItem(STALE_CHUNK_KEY); + return m; + }, + (err) => { + // A tab opened before an upgrade still runs the old bundle; the hashed chunk + // names it asks for no longer exist on the server, so reload once to pick up + // the new index.html instead of rendering the 50X error page. + if (!sessionStorage.getItem(STALE_CHUNK_KEY)) { + sessionStorage.setItem(STALE_CHUNK_KEY, '1'); + window.location.reload(); + return new Promise(() => {}); + } + throw err; + }, + ), + ); } else { Ctrl = rn.page; }