fix(web): detect incomplete embedded assets and serve error page#323
fix(web): detect incomplete embedded assets and serve error page#323ptone wants to merge 2 commits into
Conversation
When the binary is built without running 'make web' first, the embedded dist/client directory exists but only contains .gitkeep. Previously this caused a blank page because AssetsEmbedded was true and the sub-FS was non-nil, so the SPA shell referenced main.js which 404'd silently. Now NewWebServer validates that assets/main.js exists in the embedded FS after creating it. If missing, ws.assets is left nil so the existing noAssetsPage fallback kicks in, explaining how to rebuild with assets.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the detection of incomplete embedded web assets by checking for the presence of 'assets/main.js' within the embedded filesystem, updating warning messages, and updating tests to mock this file. Feedback on the changes suggests removing a redundant filesystem check and duplicate warning log in 'cmd/server_foreground.go', as these checks are already performed inside 'NewWebServer'.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if !web.AssetsEmbedded && webAssetsDir == "" { | ||
| slog.Warn("This binary was built without web assets. The web UI will not be available. Run 'make web' and rebuild to include the web frontend, or use --web-assets-dir.") | ||
| slog.Warn("This binary was built without web assets. The web UI will not be available. Run 'make all' (or 'make web && make build') to rebuild with web assets included, or use --web-assets-dir.") | ||
| } else if web.AssetsEmbedded && webAssetsDir == "" { | ||
| if sub, err := fs.Sub(web.ClientAssets, "dist/client"); err == nil { | ||
| if _, err := fs.Stat(sub, "assets/main.js"); err != nil { | ||
| slog.Warn("Embedded web assets are incomplete (main.js missing). The web UI will not be available. Run 'make all' (or 'make web && make build') to rebuild with web assets included, or use --web-assets-dir.") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This block introduces redundant file system checks and duplicate warning logs.
When enableWeb is true, initWebServer is called on line 326, which instantiates the web server via NewWebServer. Inside NewWebServer (in pkg/hub/web.go), the exact same checks (fs.Sub and fs.Stat for assets/main.js) are already performed, and appropriate warnings/errors are logged.
Therefore, executing these checks and logging another warning here is redundant and leads to duplicate warning messages in the console at startup. We can safely remove this entire block.
Note: Removing this block will make the "io/fs" import unused, so it should be removed from the import block as well.
When fs.Sub fails to create a sub-filesystem from embedded web assets, the error message now includes actionable rebuild instructions (run make web && make build) so users know how to resolve the issue.
1 participant
Summary
make web, the embeddeddist/clientdirectory exists but only has.gitkeep. Previously this caused a blank page becauseAssetsEmbeddedwas true and the sub-FS was non-nil, so the SPA shell referencedmain.jswhich 404'd silently.NewWebServervalidates thatassets/main.jsexists in the embedded FS. If missing,ws.assetsis left nil so the existingnoAssetsPagefallback renders, explaining how to rebuild with assets.make all(ormake web && make build).server_foreground.gofor the incomplete-assets case.Test plan
go build ./...compiles successfully