From 020ad9d4687eb66a197282cec8a56728ef9119af Mon Sep 17 00:00:00 2001 From: RonniSkansing Date: Tue, 15 Sep 2026 19:38:02 +0200 Subject: [PATCH] fix improve error message when browser start fails Signed-off-by: RonniSkansing --- backend/controller/utils.go | 23 +++++++++++ backend/errs/all.go | 38 +++++++++++++++++++ backend/remotebrowser/pdf.go | 16 ++++++-- frontend/src/lib/api/api.js | 30 ++++++++++++++- .../src/routes/campaign/[id]/+page.svelte | 3 +- 5 files changed, 104 insertions(+), 6 deletions(-) diff --git a/backend/controller/utils.go b/backend/controller/utils.go index f8697230..23c0459d 100644 --- a/backend/controller/utils.go +++ b/backend/controller/utils.go @@ -156,6 +156,12 @@ func (c *Common) handleErrors( ) return false } + if ok := handleOperationalError(g, c.Response, err); !ok { + c.Logger.Errorw("operational error", + "error", err, + ) + return false + } if ok := handleDBRowNotFound(g, c.Response, err); !ok { c.Logger.Debugw("DB row not found error", "error", err, @@ -308,6 +314,23 @@ func handleCustomError( return true } +// handleOperationalError responds with a 500 that carries the safe message +// from an OperationalError. The full cause is logged by the caller. It is used +// for environment failures such as a blocked download or a browser that will +// not start, so the operator sees what went wrong instead of a generic error. +func handleOperationalError( + g *gin.Context, + responseHandler api.JSONResponseHandler, + err error, +) bool { + var opErr errs.OperationalError + if errors.As(err, &opErr) { + responseHandler.ServerErrorMessage(g, opErr.PublicMessage()) + return false + } + return true +} + // handleServerError checks if the error is a server error // if it is, a 500 response is sent // if it is not, true is returned diff --git a/backend/errs/all.go b/backend/errs/all.go index 56bb6f03..ff4b1abc 100644 --- a/backend/errs/all.go +++ b/backend/errs/all.go @@ -158,3 +158,41 @@ func NewCustomError(err error) error { func (e CustomError) Error() string { return e.Err.Error() } + +// OperationalError is returned when an action fails for an environment or +// infrastructure reason that the operator should see, such as a blocked +// outbound download or a browser that will not start. The cause stays in the +// server logs while a safe message is shown to the client. +type OperationalError struct { + // public is the message shown to the client + public string + // cause is the underlying error kept for the logs + cause error +} + +// NewOperationalError creates an operational error with a message for the +// client and the underlying cause for the logs. +func NewOperationalError(public string, cause error) error { + return OperationalError{ + public: public, + cause: cause, + } +} + +// Error returns the message and the cause for the logs. +func (e OperationalError) Error() string { + if e.cause == nil { + return e.public + } + return e.public + ": " + e.cause.Error() +} + +// PublicMessage returns the message shown to the client. +func (e OperationalError) PublicMessage() string { + return e.public +} + +// Unwrap returns the underlying cause. +func (e OperationalError) Unwrap() error { + return e.cause +} diff --git a/backend/remotebrowser/pdf.go b/backend/remotebrowser/pdf.go index c20c0364..5e2584dd 100644 --- a/backend/remotebrowser/pdf.go +++ b/backend/remotebrowser/pdf.go @@ -11,6 +11,7 @@ import ( "github.com/go-rod/rod" "github.com/go-rod/rod/lib/launcher" "github.com/go-rod/rod/lib/proto" + "github.com/phishingclub/phishingclub/errs" ) // WipeBrowserCache removes the automatically downloaded Chromium directory. @@ -86,7 +87,10 @@ func RenderHTMLToPDF(ctx context.Context, htmlContent string, execPath string) ( binPath := b.BinPath() if _, err := os.Stat(binPath); os.IsNotExist(err) { if err := b.Download(); err != nil { - return nil, fmt.Errorf("reportpdf: browser download failed: %w", err) + return nil, errs.NewOperationalError( + "The report browser could not be downloaded. Check the server logs for details.", + err, + ) } } l = l.Bin(binPath) @@ -98,13 +102,19 @@ func RenderHTMLToPDF(ctx context.Context, htmlContent string, execPath string) ( // on an early launch failure it blocks forever waiting on the exit channel. // Kill still removes a process that did start. l.Kill() - return nil, fmt.Errorf("reportpdf: browser launch failed: %w", err) + return nil, errs.NewOperationalError( + "The report browser failed to start. Check the server logs for details.", + err, + ) } defer func() { l.Kill(); l.Cleanup() }() browser := rod.New().ControlURL(u).Context(ctx) if err := browser.Connect(); err != nil { - return nil, fmt.Errorf("reportpdf: browser connect failed: %w", err) + return nil, errs.NewOperationalError( + "The report browser failed to start. Check the server logs for details.", + err, + ) } defer browser.Close() //nolint:errcheck diff --git a/frontend/src/lib/api/api.js b/frontend/src/lib/api/api.js index 494f9b8f..208eec71 100644 --- a/frontend/src/lib/api/api.js +++ b/frontend/src/lib/api/api.js @@ -511,7 +511,35 @@ export class API { * @param {string} campaignID */ generateReport: async (campaignID) => { - window.open(this.getPath(`/campaign/${campaignID}/report`), '_blank'); + const response = await fetch(this.getPath(`/campaign/${campaignID}/report`), { + method: 'GET', + credentials: 'same-origin' + }); + if (!response.ok) { + let error = 'Failed to generate campaign report'; + try { + const body = await response.json(); + if (body?.error) { + error = body.error; + } + } catch (_) { + // the error body was not JSON, keep the default message + } + return { success: false, error }; + } + const blob = await response.blob(); + const disposition = response.headers.get('Content-Disposition') || ''; + const match = disposition.match(/filename="?([^"]+)"?/); + const filename = match ? match[1] : 'report.pdf'; + const url = URL.createObjectURL(blob); + const link = document.createElement('a'); + link.href = url; + link.download = filename; + document.body.appendChild(link); + link.click(); + link.remove(); + URL.revokeObjectURL(url); + return { success: true }; }, /** diff --git a/frontend/src/routes/campaign/[id]/+page.svelte b/frontend/src/routes/campaign/[id]/+page.svelte index 23aaf7ee..5e5825b5 100644 --- a/frontend/src/routes/campaign/[id]/+page.svelte +++ b/frontend/src/routes/campaign/[id]/+page.svelte @@ -1231,8 +1231,7 @@ const onConfirmGenerateReport = async () => { try { - api.campaign.generateReport($page.params.id); - return { success: true }; + return await api.campaign.generateReport($page.params.id); } catch (e) { console.error('failed to generate campaign report', e); return { success: false, error: 'Failed to generate campaign report' };