From 5bb42fa217872938681f3bbf529a364455a0d1b5 Mon Sep 17 00:00:00 2001 From: David Moore Date: Tue, 8 Apr 2025 16:17:24 +1000 Subject: [PATCH 1/5] 404.html should be the default error page --- pkg/project/project.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/project/project.go b/pkg/project/project.go index 8ffcf674..8a69d621 100644 --- a/pkg/project/project.go +++ b/pkg/project/project.go @@ -827,7 +827,7 @@ func fromProjectConfiguration(projectConfig *ProjectConfiguration, localConfig * } if websiteSpec.ErrorPage == "" { - websiteSpec.ErrorPage = "index.html" + websiteSpec.ErrorPage = "404.html" } else if !strings.HasSuffix(websiteSpec.ErrorPage, ".html") { return nil, fmt.Errorf("invalid error page %s, must end with .html", websiteSpec.ErrorPage) } From 69e7440b5ef9507c4dd8ae5b7de60ae6c1ddd154 Mon Sep 17 00:00:00 2001 From: David Moore Date: Tue, 8 Apr 2025 16:18:32 +1000 Subject: [PATCH 2/5] fix: correctly serves websites on start/run Ensures websites function correctly in both `nitric start` and `nitric run` modes. Introduces separate servers for each website when using `nitric start`, allowing them to run on individual ports. This resolves issues where websites were not being served correctly in development environments. For static serving (`nitric run`), it uses a single server with a shared port for all websites. Also, correctly strips the base path from the request URL when proxying requests to the target URL. --- pkg/cloud/websites/websites.go | 164 +++++++++++++++++++++++++-------- 1 file changed, 128 insertions(+), 36 deletions(-) diff --git a/pkg/cloud/websites/websites.go b/pkg/cloud/websites/websites.go index 69ebbebe..c5966fd9 100644 --- a/pkg/cloud/websites/websites.go +++ b/pkg/cloud/websites/websites.go @@ -75,12 +75,12 @@ func (l *LocalWebsiteService) SubscribeToState(fn func(State)) { } // register - Register a new website -func (l *LocalWebsiteService) register(website Website) { +func (l *LocalWebsiteService) register(website Website, port int) { l.websiteRegLock.Lock() defer l.websiteRegLock.Unlock() // Emulates the CDN URL used in a deployed environment - publicUrl := fmt.Sprintf("http://localhost:%d/%s", l.port, strings.TrimPrefix(website.BasePath, "/")) + publicUrl := fmt.Sprintf("http://localhost:%d/%s", port, strings.TrimPrefix(website.BasePath, "/")) l.state[website.Name] = Website{ WebsitePb: website.WebsitePb, @@ -98,6 +98,7 @@ type staticSiteHandler struct { port int devURL string isStartCmd bool + server *http.Server } func (h staticSiteHandler) serveProxy(res http.ResponseWriter, req *http.Request) { @@ -117,6 +118,17 @@ func (h staticSiteHandler) serveProxy(res http.ResponseWriter, req *http.Request return } + // Strip the base path from the request path before proxying + if h.website.BasePath != "/" { + // redirect to base if path is / and there is no query string + if req.RequestURI == "/" { + http.Redirect(res, req, h.website.BasePath, http.StatusFound) + return + } + + req.URL.Path = strings.TrimPrefix(req.URL.Path, h.website.BasePath) + } + // Reverse proxy request proxy := httputil.NewSingleHostReverseProxy(targetUrl) @@ -152,7 +164,7 @@ func (h staticSiteHandler) serveStatic(res http.ResponseWriter, req *http.Reques } if fi.IsDir() { - http.ServeFile(res, req, filepath.Join(h.website.OutputDirectory, h.website.IndexDocument)) + http.ServeFile(res, req, filepath.Join(path, h.website.IndexDocument)) return } @@ -171,21 +183,9 @@ func (h staticSiteHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) h.serveStatic(res, req) } -// Start - Start the local website service -func (l *LocalWebsiteService) Start(websites []Website) error { - newLis, err := netx.GetNextListener(netx.MinPort(5000)) - if err != nil { - return err - } - - l.port = newLis.Addr().(*net.TCPAddr).Port - - _ = newLis.Close() - - mux := http.NewServeMux() - - // Register the API proxy handler - mux.HandleFunc("/api/{name}/", func(res http.ResponseWriter, req *http.Request) { +// createAPIPathHandler creates a handler for API proxy requests +func (l *LocalWebsiteService) createAPIPathHandler() http.HandlerFunc { + return func(res http.ResponseWriter, req *http.Request) { apiName := req.PathValue("name") apiAddress := l.getApiAddress(apiName) @@ -201,31 +201,123 @@ func (l *LocalWebsiteService) Start(websites []Website) error { req.URL.Path = targetPath proxy.ServeHTTP(res, req) - }) - - // Register the SPA handler for each website - for i := range websites { - website := &websites[i] - spa := staticSiteHandler{website: website, port: l.port, devURL: website.DevURL, isStartCmd: l.isStartCmd} + } +} - if website.BasePath == "/" { - mux.Handle("/", spa) - } else { - mux.Handle(website.BasePath+"/", http.StripPrefix(website.BasePath+"/", spa)) - } +// createServer creates and configures an HTTP server with the given mux +func (l *LocalWebsiteService) createServer(mux *http.ServeMux, port int) *http.Server { + return &http.Server{ + Addr: fmt.Sprintf(":%d", port), + Handler: mux, } +} - // Start the server with the multiplexer +// startServer starts the given server in a goroutine and handles errors +func (l *LocalWebsiteService) startServer(server *http.Server, errChan chan error, errMsg string) { go func() { - addr := fmt.Sprintf(":%d", l.port) - if err := http.ListenAndServe(addr, mux); err != nil { - fmt.Printf("Failed to start server: %s\n", err) + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + select { + case errChan <- fmt.Errorf(errMsg, err): + default: + } } }() +} + +// Start - Start the local website service +func (l *LocalWebsiteService) Start(websites []Website) error { + var errChan = make(chan error, 1) + var startPort = 5000 + + if l.isStartCmd { + // In start mode, create individual servers for each website + for i := range websites { + website := &websites[i] + + // Get a new listener for each website, incrementing the port each time + newLis, err := netx.GetNextListener(netx.MinPort(startPort + i)) + if err != nil { + return err + } + + port := newLis.Addr().(*net.TCPAddr).Port + _ = newLis.Close() + + mux := http.NewServeMux() - // Register the websites - for _, website := range websites { - l.register(website) + // Register the API proxy handler for this website + mux.HandleFunc("/api/{name}/", l.createAPIPathHandler()) + + // Create the SPA handler for this website + spa := staticSiteHandler{ + website: website, + port: port, + devURL: website.DevURL, + isStartCmd: l.isStartCmd, + } + + // Register the SPA handler + mux.Handle("/", spa) + + // Create and start the server + server := l.createServer(mux, port) + + // Store the server in the handler for potential cleanup + spa.server = server + + // Register the website with its port + l.register(*website, port) + + // Start the server in a goroutine + l.startServer(server, errChan, "failed to start server for website %s: %w") + } + } else { + // For static serving, use a single server + newLis, err := netx.GetNextListener(netx.MinPort(startPort)) + if err != nil { + return err + } + + port := newLis.Addr().(*net.TCPAddr).Port + _ = newLis.Close() + + mux := http.NewServeMux() + + // Register the API proxy handler + mux.HandleFunc("/api/{name}/", l.createAPIPathHandler()) + + // Register the SPA handler for each website + for i := range websites { + website := &websites[i] + spa := staticSiteHandler{ + website: website, + port: port, + devURL: website.DevURL, + isStartCmd: l.isStartCmd, + } + + if website.BasePath == "/" { + mux.Handle("/", spa) + } else { + mux.Handle(website.BasePath+"/", http.StripPrefix(website.BasePath+"/", spa)) + } + } + + // Register all websites with the same port + for _, website := range websites { + l.register(website, port) + } + + // Create and start the server + server := l.createServer(mux, port) + + // Start the server in a goroutine + l.startServer(server, errChan, "failed to start static server: %w") + } + + // Return the first error that occurred, if any + if err := <-errChan; err != nil { + return err } return nil From 2c87c14ef6163aa2e9a36755d8e113b752780165 Mon Sep 17 00:00:00 2001 From: David Moore Date: Tue, 8 Apr 2025 16:28:25 +1000 Subject: [PATCH 3/5] fmt --- pkg/cloud/websites/websites.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/cloud/websites/websites.go b/pkg/cloud/websites/websites.go index c5966fd9..06f13f57 100644 --- a/pkg/cloud/websites/websites.go +++ b/pkg/cloud/websites/websites.go @@ -56,7 +56,6 @@ type ( type LocalWebsiteService struct { websiteRegLock sync.RWMutex state State - port int getApiAddress GetApiAddress isStartCmd bool @@ -95,7 +94,6 @@ func (l *LocalWebsiteService) register(website Website, port int) { type staticSiteHandler struct { website *Website - port int devURL string isStartCmd bool server *http.Server @@ -227,6 +225,7 @@ func (l *LocalWebsiteService) startServer(server *http.Server, errChan chan erro // Start - Start the local website service func (l *LocalWebsiteService) Start(websites []Website) error { var errChan = make(chan error, 1) + var startPort = 5000 if l.isStartCmd { @@ -251,7 +250,6 @@ func (l *LocalWebsiteService) Start(websites []Website) error { // Create the SPA handler for this website spa := staticSiteHandler{ website: website, - port: port, devURL: website.DevURL, isStartCmd: l.isStartCmd, } @@ -291,7 +289,6 @@ func (l *LocalWebsiteService) Start(websites []Website) error { website := &websites[i] spa := staticSiteHandler{ website: website, - port: port, devURL: website.DevURL, isStartCmd: l.isStartCmd, } From 3d4177273ae2c3c979c682feaee02c2057a4b60c Mon Sep 17 00:00:00 2001 From: David Moore Date: Wed, 9 Apr 2025 09:54:43 +1000 Subject: [PATCH 4/5] fix tests --- pkg/cloud/websites/websites.go | 9 +++++++-- .../frontend/cypress/e2e/websites.cy.ts | 17 ++++++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pkg/cloud/websites/websites.go b/pkg/cloud/websites/websites.go index 06f13f57..3f583f0d 100644 --- a/pkg/cloud/websites/websites.go +++ b/pkg/cloud/websites/websites.go @@ -26,6 +26,7 @@ import ( "net/url" "os" "path/filepath" + "slices" "strings" "sync" @@ -224,9 +225,13 @@ func (l *LocalWebsiteService) startServer(server *http.Server, errChan chan erro // Start - Start the local website service func (l *LocalWebsiteService) Start(websites []Website) error { - var errChan = make(chan error, 1) + errChan := make(chan error, 1) - var startPort = 5000 + startPort := 5000 + + slices.SortFunc(websites, func(a, b Website) int { + return strings.Compare(a.BasePath, b.BasePath) + }) if l.isStartCmd { // In start mode, create individual servers for each website diff --git a/pkg/dashboard/frontend/cypress/e2e/websites.cy.ts b/pkg/dashboard/frontend/cypress/e2e/websites.cy.ts index 9bf2e958..e0d5e864 100644 --- a/pkg/dashboard/frontend/cypress/e2e/websites.cy.ts +++ b/pkg/dashboard/frontend/cypress/e2e/websites.cy.ts @@ -20,17 +20,20 @@ describe('Websites Spec', () => { cy.get(`[data-rct-item-id="${id}"]`).click() cy.get('h2').should('contain.text', id) - const pathMap = { - 'vite-website': '', - 'docs-website': 'docs', + const originMap = { + 'vite-website': 'http://localhost:5000', + 'docs-website': 'http://localhost:5001', } - const url = `http://localhost:5000/${pathMap[id]}` + const pathMap = { + 'vite-website': '/', + 'docs-website': '/docs', + } // check iframe url - cy.get('iframe').should('have.attr', 'src', url) + cy.get('iframe').should('have.attr', 'src', originMap[id] + pathMap[id]) - cy.visit(url) + cy.visit(originMap[id] + pathMap[id]) const titleMap = { 'vite-website': 'Hello Nitric!', @@ -39,7 +42,7 @@ describe('Websites Spec', () => { const title = titleMap[id] - cy.origin('http://localhost:5000', { args: { title } }, ({ title }) => { + cy.origin(originMap[id], { args: { title } }, ({ title }) => { cy.get('h1').should('have.text', title) }) }) From 540802bc7b16f4f60dbed136efa0493c5ea7a122 Mon Sep 17 00:00:00 2001 From: David Moore Date: Wed, 9 Apr 2025 10:12:16 +1000 Subject: [PATCH 5/5] include cypress env variable for run type --- .github/workflows/dashboard-run-test.yaml | 2 ++ .github/workflows/dashboard-start-test.yaml | 2 ++ pkg/dashboard/frontend/cypress/e2e/websites.cy.ts | 15 ++++++++++++--- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dashboard-run-test.yaml b/.github/workflows/dashboard-run-test.yaml index 5eb7b918..4abb55a9 100644 --- a/.github/workflows/dashboard-run-test.yaml +++ b/.github/workflows/dashboard-run-test.yaml @@ -50,6 +50,8 @@ jobs: - name: Run Tests uses: cypress-io/github-action@v5 + env: + CYPRESS_NITRIC_TEST_TYPE: "run" with: install: false wait-on: "http://localhost:49152" diff --git a/.github/workflows/dashboard-start-test.yaml b/.github/workflows/dashboard-start-test.yaml index 1fc03d4c..0ac423e6 100644 --- a/.github/workflows/dashboard-start-test.yaml +++ b/.github/workflows/dashboard-start-test.yaml @@ -64,6 +64,8 @@ jobs: wait-on-timeout: 180 working-directory: cli/pkg/dashboard/frontend browser: chrome + env: + CYPRESS_NITRIC_TEST_TYPE: "start" - uses: actions/upload-artifact@v4 if: failure() diff --git a/pkg/dashboard/frontend/cypress/e2e/websites.cy.ts b/pkg/dashboard/frontend/cypress/e2e/websites.cy.ts index e0d5e864..869f3f71 100644 --- a/pkg/dashboard/frontend/cypress/e2e/websites.cy.ts +++ b/pkg/dashboard/frontend/cypress/e2e/websites.cy.ts @@ -20,9 +20,18 @@ describe('Websites Spec', () => { cy.get(`[data-rct-item-id="${id}"]`).click() cy.get('h2').should('contain.text', id) - const originMap = { - 'vite-website': 'http://localhost:5000', - 'docs-website': 'http://localhost:5001', + let originMap = {} + + if (Cypress.env('NITRIC_TEST_TYPE') === 'run') { + originMap = { + 'vite-website': 'http://localhost:5000', + 'docs-website': 'http://localhost:5000', + } + } else { + originMap = { + 'vite-website': 'http://localhost:5000', + 'docs-website': 'http://localhost:5001', + } } const pathMap = {