From 3a0e8af6e3177ae2a13c51cf295fb42d18dc5bd8 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 8 Dec 2025 14:16:00 -0900 Subject: [PATCH] feat: add view workspace button to app error page (#20960) Closes #19984 As part of this, I refactored the error template to take in a slice of actions rather than using individual booleans and strings to control the behavior. We decided a link resolves the issue for now so that is what I added, although we may want to consider a way to start the workspace and follow the logs dynamically on that page and then show the app when finished (similar to the tasks page), or at least make the link automatically start the workspace instead of only taking you to the dashboard where you have to then start the workspace. --- coderd/idpsync/idpsync.go | 17 +++--- coderd/oauth2provider/authorize.go | 27 ++++++++- coderd/tailnet.go | 34 ++++++----- coderd/workspaceapps/errors.go | 73 ++++++++++++++++------- coderd/workspaceapps/proxy.go | 93 ++++++++++++++++++++---------- enterprise/wsproxy/wsproxy.go | 16 +++-- site/site.go | 24 +++++--- site/site_test.go | 40 ++++++++----- site/static/error.html | 16 +++-- 9 files changed, 233 insertions(+), 107 deletions(-) diff --git a/coderd/idpsync/idpsync.go b/coderd/idpsync/idpsync.go index 2772a1b1ec..c57eee123b 100644 --- a/coderd/idpsync/idpsync.go +++ b/coderd/idpsync/idpsync.go @@ -251,13 +251,16 @@ type HTTPError struct { func (e HTTPError) Write(rw http.ResponseWriter, r *http.Request) { if e.RenderStaticPage { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: e.Code, - HideStatus: true, - Title: e.Msg, - Description: e.Detail, - RetryEnabled: false, - DashboardURL: "/login", - + Status: e.Code, + HideStatus: true, + Title: e.Msg, + Description: e.Detail, + Actions: []site.Action{ + { + URL: "/login", + Text: "Back to site", + }, + }, RenderDescriptionMarkdown: e.RenderDetailMarkdown, }) return diff --git a/coderd/oauth2provider/authorize.go b/coderd/oauth2provider/authorize.go index d738e781e8..e5f6314b39 100644 --- a/coderd/oauth2provider/authorize.go +++ b/coderd/oauth2provider/authorize.go @@ -75,7 +75,18 @@ func ShowAuthorizePage(accessURL *url.URL) http.HandlerFunc { callbackURL, err := url.Parse(app.CallbackURL) if err != nil { - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{Status: http.StatusInternalServerError, HideStatus: false, Title: "Internal Server Error", Description: err.Error(), RetryEnabled: false, DashboardURL: accessURL.String(), Warnings: nil}) + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusInternalServerError, + HideStatus: false, + Title: "Internal Server Error", + Description: err.Error(), + Actions: []site.Action{ + { + URL: accessURL.String(), + Text: "Back to site", + }, + }, + }) return } @@ -85,7 +96,19 @@ func ShowAuthorizePage(accessURL *url.URL) http.HandlerFunc { for i, err := range validationErrs { errStr[i] = err.Detail } - site.RenderStaticErrorPage(rw, r, site.ErrorPageData{Status: http.StatusBadRequest, HideStatus: false, Title: "Invalid Query Parameters", Description: "One or more query parameters are missing or invalid.", RetryEnabled: false, DashboardURL: accessURL.String(), Warnings: errStr}) + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusBadRequest, + HideStatus: false, + Title: "Invalid Query Parameters", + Description: "One or more query parameters are missing or invalid.", + Warnings: errStr, + Actions: []site.Action{ + { + URL: accessURL.String(), + Text: "Back to site", + }, + }, + }) return } diff --git a/coderd/tailnet.go b/coderd/tailnet.go index cdcf657fe7..c83b26e001 100644 --- a/coderd/tailnet.go +++ b/coderd/tailnet.go @@ -199,10 +199,9 @@ func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID u proxy := httputil.NewSingleHostReverseProxy(&tgt) proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, theErr error) { var ( - desc = "Failed to proxy request to application: " + theErr.Error() - additionalInfo = "" - additionalButtonLink = "" - additionalButtonText = "" + desc = "Failed to proxy request to application: " + theErr.Error() + additionalInfo = "" + actions = []site.Action{} ) var tlsError tls.RecordHeaderError @@ -222,21 +221,28 @@ func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID u app = app.ChangePortProtocol(targetProtocol) switchURL.Host = fmt.Sprintf("%s%s", app.String(), strings.TrimPrefix(wildcardHostname, "*")) - additionalButtonLink = switchURL.String() - additionalButtonText = fmt.Sprintf("Switch to %s", strings.ToUpper(targetProtocol)) + actions = append(actions, site.Action{ + URL: switchURL.String(), + Text: fmt.Sprintf("Switch to %s", strings.ToUpper(targetProtocol)), + }) additionalInfo += fmt.Sprintf("This error seems to be due to an app protocol mismatch, try switching to %s.", strings.ToUpper(targetProtocol)) } } site.RenderStaticErrorPage(w, r, site.ErrorPageData{ - Status: http.StatusBadGateway, - Title: "Bad Gateway", - Description: desc, - RetryEnabled: true, - DashboardURL: dashboardURL.String(), - AdditionalInfo: additionalInfo, - AdditionalButtonLink: additionalButtonLink, - AdditionalButtonText: additionalButtonText, + Status: http.StatusBadGateway, + Title: "Bad Gateway", + Description: desc, + Actions: append(actions, []site.Action{ + { + Text: "Retry", + }, + { + URL: dashboardURL.String(), + Text: "Back to site", + }, + }...), + AdditionalInfo: additionalInfo, }) } proxy.Director = s.director(agentID, proxy.Director) diff --git a/coderd/workspaceapps/errors.go b/coderd/workspaceapps/errors.go index 64d61de367..f5cbdf1ef6 100644 --- a/coderd/workspaceapps/errors.go +++ b/coderd/workspaceapps/errors.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/url" + "path" "cdr.dev/slog" "github.com/coder/coder/v2/codersdk" @@ -30,12 +31,16 @@ func WriteWorkspaceApp404(log slog.Logger, accessURL *url.URL, rw http.ResponseW } site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusNotFound, - Title: "Application Not Found", - Description: "The application or workspace you are trying to access does not exist or you do not have permission to access it.", - RetryEnabled: false, - DashboardURL: accessURL.String(), - Warnings: warnings, + Status: http.StatusNotFound, + Title: "Application Not Found", + Description: "The application or workspace you are trying to access does not exist or you do not have permission to access it.", + Warnings: warnings, + Actions: []site.Action{ + { + URL: accessURL.String(), + Text: "Back to site", + }, + }, }) } @@ -60,11 +65,15 @@ func WriteWorkspaceApp500(log slog.Logger, accessURL *url.URL, rw http.ResponseW ) site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusInternalServerError, - Title: "Internal Server Error", - Description: "An internal server error occurred.", - RetryEnabled: false, - DashboardURL: accessURL.String(), + Status: http.StatusInternalServerError, + Title: "Internal Server Error", + Description: "An internal server error occurred.", + Actions: []site.Action{ + { + URL: accessURL.String(), + Text: "Back to site", + }, + }, }) } @@ -85,11 +94,18 @@ func WriteWorkspaceAppOffline(log slog.Logger, accessURL *url.URL, rw http.Respo } site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadGateway, - Title: "Application Unavailable", - Description: msg, - RetryEnabled: true, - DashboardURL: accessURL.String(), + Status: http.StatusBadGateway, + Title: "Application Unavailable", + Description: msg, + Actions: []site.Action{ + { + Text: "Retry", + }, + { + URL: accessURL.String(), + Text: "Back to site", + }, + }, }) } @@ -109,11 +125,26 @@ func WriteWorkspaceOffline(log slog.Logger, accessURL *url.URL, rw http.Response ) } + actions := []site.Action{ + { + URL: accessURL.String(), + Text: "Back to site", + }, + } + + workspaceURL, err := url.Parse(accessURL.String()) + if err == nil { + workspaceURL.Path = path.Join(accessURL.Path, "@"+appReq.UsernameOrID, appReq.WorkspaceNameOrID) + actions = append(actions, site.Action{ + URL: workspaceURL.String(), + Text: "View workspace", + }) + } + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadRequest, - Title: "Workspace Offline", - Description: fmt.Sprintf("Last workspace transition was to the %q state. Start the workspace to access its applications.", codersdk.WorkspaceTransitionStop), - RetryEnabled: false, - DashboardURL: accessURL.String(), + Status: http.StatusBadRequest, + Title: "Workspace Offline", + Description: fmt.Sprintf("Last workspace transition was to the %q state. Start the workspace to access its applications.", codersdk.WorkspaceTransitionStop), + Actions: actions, }) } diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go index 981bba4584..8806841fb7 100644 --- a/coderd/workspaceapps/proxy.go +++ b/coderd/workspaceapps/proxy.go @@ -185,10 +185,14 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request, Status: http.StatusBadRequest, Title: "Bad Request", Description: "Could not decrypt API key. Workspace app API key smuggling is not permitted on the primary access URL. Please remove the query parameter and try again.", - // Retry is disabled because the user needs to remove the query + // No retry is included because the user needs to remove the query // parameter before they try again. - RetryEnabled: false, - DashboardURL: s.DashboardURL.String(), + Actions: []site.Action{ + { + URL: s.DashboardURL.String(), + Text: "Back to site", + }, + }, }) return false } @@ -204,10 +208,14 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request, Status: http.StatusBadRequest, Title: "Bad Request", Description: "Could not decrypt API key. Please remove the query parameter and try again.", - // Retry is disabled because the user needs to remove the query + // No retry is included because the user needs to remove the query // parameter before they try again. - RetryEnabled: false, - DashboardURL: s.DashboardURL.String(), + Actions: []site.Action{ + { + URL: s.DashboardURL.String(), + Text: "Back to site", + }, + }, }) return false } @@ -224,11 +232,15 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request, // startup, but we'll check anyways. s.Logger.Error(r.Context(), "could not split invalid app hostname", slog.F("hostname", s.Hostname)) site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusInternalServerError, - Title: "Internal Server Error", - Description: "The app is configured with an invalid app wildcard hostname. Please contact an administrator.", - RetryEnabled: false, - DashboardURL: s.DashboardURL.String(), + Status: http.StatusInternalServerError, + Title: "Internal Server Error", + Description: "The app is configured with an invalid app wildcard hostname. Please contact an administrator.", + Actions: []site.Action{ + { + URL: s.DashboardURL.String(), + Text: "Back to site", + }, + }, }) return false } @@ -274,11 +286,15 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request, func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) { if s.DisablePathApps { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusForbidden, - Title: "Forbidden", - Description: "Path-based applications are disabled on this Coder deployment by the administrator.", - RetryEnabled: false, - DashboardURL: s.DashboardURL.String(), + Status: http.StatusForbidden, + Title: "Forbidden", + Description: "Path-based applications are disabled on this Coder deployment by the administrator.", + Actions: []site.Action{ + { + URL: s.DashboardURL.String(), + Text: "Back to site", + }, + }, }) return } @@ -287,11 +303,15 @@ func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) // lookup the username from token. We used to redirect by doing this lookup. if chi.URLParam(r, "user") == codersdk.Me { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusNotFound, - Title: "Application Not Found", - Description: "Applications must be accessed with the full username, not @me.", - RetryEnabled: false, - DashboardURL: s.DashboardURL.String(), + Status: http.StatusNotFound, + Title: "Application Not Found", + Description: "Applications must be accessed with the full username, not @me.", + Actions: []site.Action{ + { + URL: s.DashboardURL.String(), + Text: "Back to site", + }, + }, }) return } @@ -519,11 +539,15 @@ func (s *Server) parseHostname(rw http.ResponseWriter, r *http.Request, next htt app, err := appurl.ParseSubdomainAppURL(subdomain) if err != nil { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadRequest, - Title: "Invalid Application URL", - Description: fmt.Sprintf("Could not parse subdomain application URL %q: %s", subdomain, err.Error()), - RetryEnabled: false, - DashboardURL: s.DashboardURL.String(), + Status: http.StatusBadRequest, + Title: "Invalid Application URL", + Description: fmt.Sprintf("Could not parse subdomain application URL %q: %s", subdomain, err.Error()), + Actions: []site.Action{ + { + URL: s.DashboardURL.String(), + Text: "Back to site", + }, + }, }) return appurl.ApplicationURL{}, false } @@ -547,11 +571,18 @@ func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appT appURL, err := url.Parse(appToken.AppURL) if err != nil { site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ - Status: http.StatusBadRequest, - Title: "Bad Request", - Description: fmt.Sprintf("Application has an invalid URL %q: %s", appToken.AppURL, err.Error()), - RetryEnabled: true, - DashboardURL: s.DashboardURL.String(), + Status: http.StatusBadRequest, + Title: "Bad Request", + Description: fmt.Sprintf("Application has an invalid URL %q: %s", appToken.AppURL, err.Error()), + Actions: []site.Action{ + { + Text: "Retry", + }, + { + URL: s.DashboardURL.String(), + Text: "Back to site", + }, + }, }) return } diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go index ecd5df9cde..cfaa472242 100644 --- a/enterprise/wsproxy/wsproxy.go +++ b/enterprise/wsproxy/wsproxy.go @@ -379,8 +379,12 @@ func New(ctx context.Context, opts *Options) (*Server, error) { HideStatus: true, Description: "This workspace proxy is DERP-only and cannot be used for browser connections. " + "Please use a different region directly from the dashboard. Click to be redirected!", - RetryEnabled: false, - DashboardURL: opts.DashboardURL.String(), + Actions: []site.Action{ + { + URL: opts.DashboardURL.String(), + Text: "Back to site", + }, + }, }) } serveDerpOnlyHandler := func(r chi.Router) { @@ -422,8 +426,12 @@ func New(ctx context.Context, opts *Options) (*Server, error) { HideStatus: true, Description: "Workspace Proxies route traffic in terminals and apps directly to your workspace. " + "This page must be loaded from the dashboard. Click to be redirected!", - RetryEnabled: false, - DashboardURL: opts.DashboardURL.String(), + Actions: []site.Action{ + { + URL: opts.DashboardURL.String(), + Text: "Back to site", + }, + }, }) }) diff --git a/site/site.go b/site/site.go index b91bde14cc..50b261ea72 100644 --- a/site/site.go +++ b/site/site.go @@ -933,20 +933,26 @@ func extractBin(dest string, r io.Reader) (numExtracted int, err error) { } } +// Action represents a link. +type Action struct { + // URL is set as the href property on the anchor. If empty, refreshes the + // page instead. + URL string + // Text is the displayed text of the button or link. + Text string +} + // ErrorPageData contains the variables that are found in // site/static/error.html. type ErrorPageData struct { Status int // HideStatus will remove the status code from the page. - HideStatus bool - Title string - Description string - RetryEnabled bool - DashboardURL string - Warnings []string - AdditionalInfo string - AdditionalButtonLink string - AdditionalButtonText string + HideStatus bool + Title string + Description string + Actions []Action + Warnings []string + AdditionalInfo string RenderDescriptionMarkdown bool } diff --git a/site/site_test.go b/site/site_test.go index 36ec124ef8..4491c75af8 100644 --- a/site/site_test.go +++ b/site/site_test.go @@ -676,11 +676,18 @@ func TestRenderStaticErrorPage(t *testing.T) { t.Parallel() d := site.ErrorPageData{ - Status: http.StatusBadGateway, - Title: "Bad Gateway 1234", - Description: "shout out colin", - RetryEnabled: true, - DashboardURL: "https://example.com", + Status: http.StatusBadGateway, + Title: "Bad Gateway 1234", + Description: "shout out colin", + Actions: []site.Action{ + { + Text: "Retry", + }, + { + URL: "https://example.com", + Text: "Back to site", + }, + }, } rw := httptest.NewRecorder() @@ -699,19 +706,26 @@ func TestRenderStaticErrorPage(t *testing.T) { require.Contains(t, bodyStr, d.Title) require.Contains(t, bodyStr, d.Description) require.Contains(t, bodyStr, "Retry") - require.Contains(t, bodyStr, d.DashboardURL) + require.Contains(t, bodyStr, "https://example.com") } func TestRenderStaticErrorPageNoStatus(t *testing.T) { t.Parallel() d := site.ErrorPageData{ - HideStatus: true, - Status: http.StatusBadGateway, - Title: "Bad Gateway 1234", - Description: "shout out colin", - RetryEnabled: true, - DashboardURL: "https://example.com", + HideStatus: true, + Status: http.StatusBadGateway, + Title: "Bad Gateway 1234", + Description: "shout out colin", + Actions: []site.Action{ + { + Text: "Retry", + }, + { + URL: "https://example.com", + Text: "Back to site", + }, + }, } rw := httptest.NewRecorder() @@ -730,7 +744,7 @@ func TestRenderStaticErrorPageNoStatus(t *testing.T) { require.Contains(t, bodyStr, d.Title) require.Contains(t, bodyStr, d.Description) require.Contains(t, bodyStr, "Retry") - require.Contains(t, bodyStr, d.DashboardURL) + require.Contains(t, bodyStr, "https://example.com") } func TestJustFilesSystem(t *testing.T) { diff --git a/site/static/error.html b/site/static/error.html index bd73ed808f..22ea4d6f58 100644 --- a/site/static/error.html +++ b/site/static/error.html @@ -187,14 +187,18 @@ {{ end }} + + {{- if .Error.Actions }}
- {{- if and .Error.AdditionalButtonText .Error.AdditionalButtonLink }} - {{ .Error.AdditionalButtonText }} - {{ end }} {{- if .Error.RetryEnabled }} - - {{ end }} - Back to site + {{ range $i, $v := .Error.Actions }} + {{- if $v.URL }} + {{ $v.Text }} + {{ else }} + + {{end}} + {{end}}
+ {{end}}