From ca20fdeb2fb8e1ccd8ffbf1f2a47cb574ddab66b Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Wed, 7 Oct 2026 08:13:55 +0200 Subject: [PATCH 1/3] fix(ui): leave JsVar rejection alerts to ClientCheck --- doc/runtime.md | 2 ++ doc/transport.md | 3 ++- doc/ui/jsvar.md | 9 +++++++-- event.go | 1 + jaws.go | 6 ++++-- lib/ui/errjsvar.go | 4 ++-- lib/ui/jsvar.go | 7 ++++++- lib/ui/jsvar_input_test.go | 39 +++++++++++++++++++++++++++++++++----- lib/ui/jsvar_store_test.go | 4 ++++ lib/what/what.go | 4 ++-- requestloop.go | 24 ++++++++++++++++------- 11 files changed, 81 insertions(+), 22 deletions(-) diff --git a/doc/runtime.md b/doc/runtime.md index 2a5e0686..c05aa6ec 100644 --- a/doc/runtime.md +++ b/doc/runtime.md @@ -63,6 +63,8 @@ Register event handlers during rendering, before the Element is frozen. Dispatch tries attached handlers in reverse registration order, then the Element's UI. Return `jaws.ErrEventUnhandled` to try the next handler. Other errors are logged and, when possible, displayed as browser danger alerts. +JavaScript store rejections are logged and corrected without automatic alerts; +the application's `ClientCheck` can send its own [feedback](ui/jsvar.md). Recovered panics show a generic alert while logs retain details. If no handler accepts an event, it is ignored without an error or alert. diff --git a/doc/transport.md b/doc/transport.md index 375b1a5a..3daba153 100644 --- a/doc/transport.md +++ b/doc/transport.md @@ -117,7 +117,8 @@ known to the Request are removed from server bookkeeping. `Hook` is a synchronous test event. The browser never sends it, and inbound messages do not dispatch it. A broadcast Hook invokes a matching handler; that -handler must not send its own messages. Its returned error becomes an alert. +handler must not send its own messages. Returned errors follow the +[event alert policy](runtime.md#connect-and-handle-events). ## Limits and connection loops diff --git a/doc/ui/jsvar.md b/doc/ui/jsvar.md index d96c923f..344a30cf 100644 --- a/doc/ui/jsvar.md +++ b/doc/ui/jsvar.md @@ -39,8 +39,13 @@ complete tentative Go value under the store's write lock. Validate authorization through the originating Element and validate the full value, including fields omitted from JSON and changes through parent or root paths. A `null` proposal for a struct zeroes all its fields, including unexported and `json:"-"` fields. An -error or panic rolls the proposal back. The callback must only inspect: do not -mutate or retain tentative data, reacquire the lock, or call a store setter. +error or panic rolls the proposal back. The callback must only inspect tentative +data: do not mutate or retain it, reacquire the lock, or call a store setter. + +Rejected proposals are logged and corrected without an automatic browser alert. +When handling browser input, `ClientCheck` can call +`source.Request.Alert("warning", "Choose a valid value.")` before returning an +error. A panic still produces a generic event-handler alert. Every binding receives the same JSON value; use separate stores for data with different visibility. Proposal outcomes can depend on Go fields omitted from diff --git a/event.go b/event.go index 7cb6ec92..76f92cc4 100644 --- a/event.go +++ b/event.go @@ -147,6 +147,7 @@ type InputHandler interface { // // [ErrEventUnhandled] tries the next handler. Other non-nil errors are logged // and queued as danger alerts when possible during Request event processing. + // JsVar store rejections are logged without automatic alerts. JawsInput(elem *Element, value string) (err error) } diff --git a/jaws.go b/jaws.go index 894933f7..4ea1c0e0 100644 --- a/jaws.go +++ b/jaws.go @@ -12,8 +12,10 @@ package jaws // must not block or call back into the same Request. // // Bound-value locks in lib/bind, lib/ui, and lib/named are released before dirtying or -// broadcasting. InitialHTMLAttrHandler callbacks run without caller-held widget or -// bound-value locks; bind.Binder acquires its own lock before calling its narrower +// broadcasting, except that a JsVar ClientCheck may send an explicit Request.Alert. +// Broadcast delivery does not acquire bound-value locks or wait for Requests. +// InitialHTMLAttrHandler callbacks run without caller-held widget or bound-value +// locks; bind.Binder acquires its own lock before calling its narrower // InitialHTMLAttrHook. Code holding a core lock must not invoke UI value methods. // Container reconciliation has one deliberate reverse edge: containerState.mu may be // held while Request.NewElement takes Request.mu. Provider callbacks and validation diff --git a/lib/ui/errjsvar.go b/lib/ui/errjsvar.go index 2cf4b7d4..3e8782cd 100644 --- a/lib/ui/errjsvar.go +++ b/lib/ui/errjsvar.go @@ -39,5 +39,5 @@ func (e errJsVarClientWrite) Is(target error) bool { return target != jaws.ErrEventUnhandled && errors.Is(e.cause, target) } -// JawsClientAlert returns a generic browser message for a rejected proposal. -func (errJsVarClientWrite) JawsClientAlert() string { return "invalid JsVar update" } +// JawsClientAlert suppresses automatic alerts for rejected proposals. +func (errJsVarClientWrite) JawsClientAlert() string { return "" } diff --git a/lib/ui/jsvar.go b/lib/ui/jsvar.go index c5bd4bde..c629fc94 100644 --- a/lib/ui/jsvar.go +++ b/lib/ui/jsvar.go @@ -88,6 +88,11 @@ func validateJsVarPath(path string) error { // and changes through parent or root paths. A null proposal for a struct zeroes // all its fields, including unexported and json:"-" fields. The check sees // converted Go values, not raw browser JSON; see [JsVarBinding.JawsInput]. +// +// Rejection errors are logged during event processing without automatic browser +// alerts. When handling browser input, the check may call [jaws.Request.Alert] +// on source.Request to provide user-facing feedback. An error matching +// [ErrJsVarTooLarge] cancels the Request. type JsVarCheck[T any] func(source *jaws.Element, next *T, path string) error // JSONSizeCheck limits the encoded size of a tentative JsVar store value. @@ -586,6 +591,7 @@ func (store *JsVarStore[T]) projectVisiblePatch(path, root string, visible any) // A rejected, invalid, or unchanged proposal schedules a canonical correction // for its source binding. A changed accepted proposal invalidates every binding. // An unchanged proposal to a complex Go shape is rejected. +// Returned errors are logged during event processing without automatic alerts. // A panicking check rolls back and schedules a root correction before the panic // continues. [ErrJsVarTooLarge] cancels the source Request for reload recovery. // @@ -629,7 +635,6 @@ func (binding *JsVarBinding[T]) JawsInput(elem *jaws.Element, input string) (err if err != nil { if errors.Is(err, ErrJsVarTooLarge) { elem.Request.Cancel(err) - return ErrJsVarTooLarge } return errJsVarClientWrite{err} } diff --git a/lib/ui/jsvar_input_test.go b/lib/ui/jsvar_input_test.go index f5e0e558..515a4243 100644 --- a/lib/ui/jsvar_input_test.go +++ b/lib/ui/jsvar_input_test.go @@ -23,6 +23,9 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { name string input string checkError error + alert string + hook bool + readOnly bool wantPatch string wantChecks int }{ @@ -30,8 +33,11 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { {name: "invalid path", input: "value..field=2", wantPatch: `={"value":1}`}, {name: "missing path", input: "missing=2", wantPatch: "missing="}, {name: "invalid JSON", input: "value={", wantPatch: `={"value":1}`}, + {name: "read-only store", input: "value=2", checkError: ErrJsVarReadOnly, readOnly: true, wantPatch: "value=1"}, {name: "rejected check", input: "value=2", checkError: errors.New("private check detail"), wantPatch: "value=1", wantChecks: 1}, {name: "unhandled check", input: "value=2", checkError: fmt.Errorf("private check detail: %w", jaws.ErrEventUnhandled), wantPatch: "value=1", wantChecks: 1}, + {name: "explicit alert", input: "value=2", checkError: errors.New("private check detail"), alert: "Choose a value no greater than 1.", wantPatch: "value=1", wantChecks: 1}, + {name: "hook rejection", input: "value=2", checkError: errors.New("private check detail"), hook: true, wantPatch: "value=1", wantChecks: 1}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -40,6 +46,8 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { if err != nil { t.Fatal(err) } + logger := new(templateLogger) + jw.Logger = logger go jw.Serve() tr := jawstest.NewTestRequest(jw, nil) <-tr.ReadyCh @@ -55,20 +63,41 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { }{Value: 1} store := newTestJsVarStore(t, jw, "client", &mu, &state) checks := 0 - store.ClientCheck = func(*jaws.Element, *struct { + store.ClientCheck = func(source *jaws.Element, _ *struct { Value int `json:"value"` - }, string, + }, _ string, ) error { checks++ + if tt.alert != "" { + source.Request.Alert("warning", tt.alert) + } return tt.checkError } + if tt.readOnly { + store.ClientCheck = nil + } _, elem, _ := renderTestJsVar(t, tr.Request, store) - tr.InCh <- wire.WsMsg{Jid: elem.Jid(), What: what.JsVar, Data: tt.input} + if tt.hook { + tr.BcastCh <- wire.Message{Dest: store, What: what.Hook, Data: tt.input} + } else { + tr.InCh <- wire.WsMsg{Jid: elem.Jid(), What: what.JsVar, Data: tt.input} + } synctest.Wait() time.Sleep(jaws.DefaultUpdateInterval + time.Millisecond) synctest.Wait() + logged := logger.sync(t, jw) + if len(logged) != 1 { + t.Fatalf("logged errors = %v, want one rejection", logged) + } + if tt.checkError != nil && logged[0].Error() != tt.checkError.Error() { + t.Fatalf("logged error = %q, want %q", logged[0], tt.checkError) + } var alerts, patches int + var wantAlerts int + if tt.alert != "" { + wantAlerts = 1 + } for { select { case msg, ok := <-tr.OutCh: @@ -78,7 +107,7 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { switch msg.What { case what.Alert: alerts++ - if msg.Jid != 0 || msg.Data != "danger\ninvalid JsVar update" { + if tt.alert == "" || msg.Jid != 0 || msg.Data != "warning\n"+tt.alert { t.Fatalf("browser alert = %#v", msg) } case what.JsVar: @@ -90,7 +119,7 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { t.Fatalf("unexpected browser message: %#v", msg) } default: - if alerts != 1 || patches != 1 || checks != tt.wantChecks || state.Value != 1 { + if alerts != wantAlerts || patches != 1 || checks != tt.wantChecks || state.Value != 1 { t.Fatalf("alerts=%d patches=%d checks=%d value=%d", alerts, patches, checks, state.Value) } return diff --git a/lib/ui/jsvar_store_test.go b/lib/ui/jsvar_store_test.go index b2f38f52..a5c9a06b 100644 --- a/lib/ui/jsvar_store_test.go +++ b/lib/ui/jsvar_store_test.go @@ -128,6 +128,10 @@ func TestJsVarStoreJSONSizeRollback(t *testing.T) { if !errors.Is(err, ErrJsVarTooLarge) || state.Text != "ok" { t.Fatalf("oversize proposal: state=%q err=%v", state.Text, err) } + var silent errJsVarClientWrite + if !errors.As(err, &silent) || silent.JawsClientAlert() != "" { + t.Fatalf("oversize proposal error permits an automatic alert: %v", err) + } if rq.Context().Err() == nil { t.Fatal("oversize proposal did not cancel its Request") } diff --git a/lib/what/what.go b/lib/what/what.go index 66f30669..4d9846da 100644 --- a/lib/what/what.go +++ b/lib/what/what.go @@ -73,8 +73,8 @@ const ( // the JaWS client never sends it, and inbound client messages are never // dispatched as Hook. Broadcasting a Hook message lets a test drive an // element's event handler synchronously, without round-tripping through the - // client. The handler must not send messages of its own; any error it - // returns is delivered to the client as an [Alert]. + // client. The handler must not send messages of its own. Returned errors are + // logged and normally sent as [Alert] messages; JsVar store rejections omit alerts. Hook ) diff --git a/requestloop.go b/requestloop.go index b2275535..8bfd5d61 100644 --- a/requestloop.go +++ b/requestloop.go @@ -186,11 +186,13 @@ func (rq *Request) handleBroadcast(tagmsg wire.Message, eventCallCh chan eventFn // Hook messages synchronously invoke the element's event handler; see // [what.Hook]. They exist for testing: the JaWS client never sends Hook, // so they only arrive here via Broadcast. The handler must not send any - // messages itself, but may return an error, which is sent back to the - // client as an alert message. + // messages itself. Returned errors follow the event alert policy. if err := rq.Jaws.Log(rq.callAllEventHandlers(elem.Jid(), tagmsg.What, tagmsg.Data)); err != nil { + if err = eventAlertError(err); err == nil { + continue + } var m wire.WsMsg - m.FillAlert(eventAlertError(err)) + m.FillAlert(err) m.Jid = elem.Jid() rq.queue(m) } @@ -541,8 +543,12 @@ func (rq *Request) eventCaller(eventCallCh <-chan eventFnCall, outboundMsgCh cha default: } if err := rq.Jaws.Log(call.invoke()); err != nil { + alertErr := eventAlertError(err) + if alertErr == nil { + continue + } var m wire.WsMsg - m.FillAlert(eventAlertError(err)) + m.FillAlert(alertErr) // This error alert is best-effort: unlike queueEvent, which cancels the // Request with ErrRequestOverloaded when its channel fills (dropping a // queued event could desync browser and backend state), a dropped alert @@ -558,16 +564,20 @@ func (rq *Request) eventCaller(eventCallCh <-chan eventFnCall, outboundMsgCh cha } } -// clientAlertError is an internal marker for library errors with safe browser text. +// clientAlertError selects safe browser text for library errors. +// An empty message suppresses the automatic alert, leaving logging unchanged. type clientAlertError interface { JawsClientAlert() string } -// eventAlertError limits browser alerts for library errors that carry internal detail. +// eventAlertError returns the browser-safe error, or nil to suppress the alert. func eventAlertError(err error) error { var safe clientAlertError if errors.As(err, &safe) { - return errors.New(safe.JawsClientAlert()) + if msg := safe.JawsClientAlert(); msg != "" { + return errors.New(msg) + } + return nil } return err } From 82e3ff0cb20924f5055fafcf6782348a7704fd55 Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Wed, 7 Oct 2026 08:29:37 +0200 Subject: [PATCH 2/3] fix: log handler panics without automatic browser alerts --- contracts.go | 2 + doc/runtime.md | 4 +- doc/ui/jsvar.md | 2 +- errors.go | 16 ++--- event.go | 3 +- lib/ui/errjsvar.go | 5 +- lib/ui/jsvar.go | 3 +- lib/ui/jsvar_input_test.go | 15 ++++- lib/ui/jsvar_store_test.go | 3 +- lib/what/what.go | 3 +- request_test.go | 123 ++++++++++++++++++++++++------------- requestloop.go | 31 +--------- 12 files changed, 122 insertions(+), 88 deletions(-) diff --git a/contracts.go b/contracts.go index abf5d532..1bbf0fa0 100644 --- a/contracts.go +++ b/contracts.go @@ -143,6 +143,7 @@ type ClickHandler interface { // // [ErrEventUnhandled] tries the next handler. Other non-nil errors are logged // and queued as danger alerts when possible during Request event processing. + // Errors matching [ErrEventLogOnly], including recovered panics, omit the alert. JawsClick(elem *Element, click Click) (err error) } @@ -160,6 +161,7 @@ type ContextMenuHandler interface { // // [ErrEventUnhandled] tries the next handler. Other non-nil errors are logged // and queued as danger alerts when possible during Request event processing. + // Errors matching [ErrEventLogOnly], including recovered panics, omit the alert. JawsContextMenu(elem *Element, click Click) (err error) } diff --git a/doc/runtime.md b/doc/runtime.md index c05aa6ec..ba5817b8 100644 --- a/doc/runtime.md +++ b/doc/runtime.md @@ -65,7 +65,9 @@ Element's UI. Return `jaws.ErrEventUnhandled` to try the next handler. Other errors are logged and, when possible, displayed as browser danger alerts. JavaScript store rejections are logged and corrected without automatic alerts; the application's `ClientCheck` can send its own [feedback](ui/jsvar.md). -Recovered panics show a generic alert while logs retain details. +Recovered panics are also logged without automatic alerts. Errors matching +`jaws.ErrEventLogOnly` suppress the automatic alert, including wrapped or joined +errors; explicit `Request.Alert` calls are unaffected. If no handler accepts an event, it is ignored without an error or alert. Browser event handlers run serially within each Request, concurrently with update diff --git a/doc/ui/jsvar.md b/doc/ui/jsvar.md index 344a30cf..0d421ae8 100644 --- a/doc/ui/jsvar.md +++ b/doc/ui/jsvar.md @@ -45,7 +45,7 @@ data: do not mutate or retain it, reacquire the lock, or call a store setter. Rejected proposals are logged and corrected without an automatic browser alert. When handling browser input, `ClientCheck` can call `source.Request.Alert("warning", "Choose a valid value.")` before returning an -error. A panic still produces a generic event-handler alert. +error. Recovered panics are also logged without automatic alerts. Every binding receives the same JSON value; use separate stores for data with different visibility. Proposal outcomes can depend on Go fields omitted from diff --git a/errors.go b/errors.go index af9f9aba..d8d9ed2f 100644 --- a/errors.go +++ b/errors.go @@ -232,7 +232,7 @@ func newErrNoWebSocketRequest(rq *Request) error { // Match it with [errors.Is]. When the recovered panic value is itself an error it is // available via Unwrap (and thus [errors.As] / [errors.Is]); a non-error panic value // appears only in the formatted message. Request event processing logs the full -// error and queues a generic browser alert. +// error without an automatic browser alert; it also matches [ErrEventLogOnly]. var ErrEventHandlerPanic errEventHandlerPanic type errEventHandlerPanic struct { @@ -247,13 +247,8 @@ func (e errEventHandlerPanic) Error() string { return fmt.Sprintf("jaws: %v panic: %v", e.Type, e.Value) } -// JawsClientAlert returns the browser message for a recovered handler panic. -func (errEventHandlerPanic) JawsClientAlert() string { - return "event handler failed" -} - func (errEventHandlerPanic) Is(target error) bool { - return target == ErrEventHandlerPanic + return target == ErrEventHandlerPanic || target == ErrEventLogOnly } func (e errEventHandlerPanic) Unwrap() error { @@ -263,6 +258,13 @@ func (e errEventHandlerPanic) Unwrap() error { return nil } +// ErrEventLogOnly marks event errors that are logged without automatic browser alerts. +// +// Request event processing suppresses an error's alert when [errors.Is] matches +// this sentinel, including wrapped or joined errors. Explicit [Request.Alert] +// calls are unaffected. +var ErrEventLogOnly = errors.New("event error is log-only") + type errEventUnhandled struct{} func (errEventUnhandled) Error() string { diff --git a/event.go b/event.go index 76f92cc4..9a362171 100644 --- a/event.go +++ b/event.go @@ -147,7 +147,8 @@ type InputHandler interface { // // [ErrEventUnhandled] tries the next handler. Other non-nil errors are logged // and queued as danger alerts when possible during Request event processing. - // JsVar store rejections are logged without automatic alerts. + // Errors matching [ErrEventLogOnly], including recovered panics and JsVar store + // rejections, are logged without automatic alerts. JawsInput(elem *Element, value string) (err error) } diff --git a/lib/ui/errjsvar.go b/lib/ui/errjsvar.go index 3e8782cd..beefbe58 100644 --- a/lib/ui/errjsvar.go +++ b/lib/ui/errjsvar.go @@ -36,8 +36,5 @@ func (e errJsVarClientWrite) Error() string { return e.cause.Error() } func (e errJsVarClientWrite) Is(target error) bool { // Event dispatch treats ErrEventUnhandled as fallthrough. A rejected proposal // is always handled, even when an application check returns that sentinel. - return target != jaws.ErrEventUnhandled && errors.Is(e.cause, target) + return target == jaws.ErrEventLogOnly || (target != jaws.ErrEventUnhandled && errors.Is(e.cause, target)) } - -// JawsClientAlert suppresses automatic alerts for rejected proposals. -func (errJsVarClientWrite) JawsClientAlert() string { return "" } diff --git a/lib/ui/jsvar.go b/lib/ui/jsvar.go index c629fc94..b8e87c2c 100644 --- a/lib/ui/jsvar.go +++ b/lib/ui/jsvar.go @@ -591,7 +591,8 @@ func (store *JsVarStore[T]) projectVisiblePatch(path, root string, visible any) // A rejected, invalid, or unchanged proposal schedules a canonical correction // for its source binding. A changed accepted proposal invalidates every binding. // An unchanged proposal to a complex Go shape is rejected. -// Returned errors are logged during event processing without automatic alerts. +// Returned errors match [jaws.ErrEventLogOnly] and are logged during event +// processing without automatic alerts. // A panicking check rolls back and schedules a root correction before the panic // continues. [ErrJsVarTooLarge] cancels the source Request for reload recovery. // diff --git a/lib/ui/jsvar_input_test.go b/lib/ui/jsvar_input_test.go index 515a4243..ce56a759 100644 --- a/lib/ui/jsvar_input_test.go +++ b/lib/ui/jsvar_input_test.go @@ -23,6 +23,7 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { name string input string checkError error + checkPanic bool alert string hook bool readOnly bool @@ -38,6 +39,8 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { {name: "unhandled check", input: "value=2", checkError: fmt.Errorf("private check detail: %w", jaws.ErrEventUnhandled), wantPatch: "value=1", wantChecks: 1}, {name: "explicit alert", input: "value=2", checkError: errors.New("private check detail"), alert: "Choose a value no greater than 1.", wantPatch: "value=1", wantChecks: 1}, {name: "hook rejection", input: "value=2", checkError: errors.New("private check detail"), hook: true, wantPatch: "value=1", wantChecks: 1}, + {name: "panicking check", input: "value=2", checkError: errors.New("private panic detail"), checkPanic: true, wantPatch: `={"value":1}`, wantChecks: 1}, + {name: "hook panic", input: "value=2", checkError: errors.New("private panic detail"), checkPanic: true, hook: true, wantPatch: `={"value":1}`, wantChecks: 1}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -71,6 +74,9 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { if tt.alert != "" { source.Request.Alert("warning", tt.alert) } + if tt.checkPanic { + panic(tt.checkError) + } return tt.checkError } if tt.readOnly { @@ -89,7 +95,14 @@ func TestJsVarRejectedBrowserInput(t *testing.T) { if len(logged) != 1 { t.Fatalf("logged errors = %v, want one rejection", logged) } - if tt.checkError != nil && logged[0].Error() != tt.checkError.Error() { + if !errors.Is(logged[0], jaws.ErrEventLogOnly) { + t.Fatalf("logged error does not match ErrEventLogOnly: %v", logged[0]) + } + if tt.checkPanic { + if !errors.Is(logged[0], jaws.ErrEventHandlerPanic) || !errors.Is(logged[0], tt.checkError) { + t.Fatalf("logged panic = %v, want %v", logged[0], tt.checkError) + } + } else if tt.checkError != nil && logged[0].Error() != tt.checkError.Error() { t.Fatalf("logged error = %q, want %q", logged[0], tt.checkError) } diff --git a/lib/ui/jsvar_store_test.go b/lib/ui/jsvar_store_test.go index a5c9a06b..cc980f3b 100644 --- a/lib/ui/jsvar_store_test.go +++ b/lib/ui/jsvar_store_test.go @@ -128,8 +128,7 @@ func TestJsVarStoreJSONSizeRollback(t *testing.T) { if !errors.Is(err, ErrJsVarTooLarge) || state.Text != "ok" { t.Fatalf("oversize proposal: state=%q err=%v", state.Text, err) } - var silent errJsVarClientWrite - if !errors.As(err, &silent) || silent.JawsClientAlert() != "" { + if !errors.Is(err, jaws.ErrEventLogOnly) { t.Fatalf("oversize proposal error permits an automatic alert: %v", err) } if rq.Context().Err() == nil { diff --git a/lib/what/what.go b/lib/what/what.go index 4d9846da..76d1caed 100644 --- a/lib/what/what.go +++ b/lib/what/what.go @@ -74,7 +74,8 @@ const ( // dispatched as Hook. Broadcasting a Hook message lets a test drive an // element's event handler synchronously, without round-tripping through the // client. The handler must not send messages of its own. Returned errors are - // logged and normally sent as [Alert] messages; JsVar store rejections omit alerts. + // logged and normally sent as [Alert] messages; recovered panics and JsVar store + // rejections omit alerts. Hook ) diff --git a/request_test.go b/request_test.go index 689ee42a..bafce7a6 100644 --- a/request_test.go +++ b/request_test.go @@ -1282,48 +1282,89 @@ func TestRequest_Trigger(t *testing.T) { } } -func TestRequest_EventHandlerPanicAlertHidesDetails(t *testing.T) { - tj := newTestJaws() - t.Cleanup(tj.Close) - rq := newWrappedTestRequest(tj.Jaws, nil) - t.Cleanup(rq.Close) - - const detail = "password authentication failed for billing_rw" - id := rq.Register(&testUi{}, func(*Element, string) error { - panic(detail) - }) - rq.InCh <- wire.WsMsg{Jid: id, What: what.Input} - - select { - case msg := <-rq.OutCh: - if msg.What != what.Alert || msg.Data != "danger\nevent handler failed" { - t.Fatalf("panic alert = %#v, want generic danger alert", msg) - } - case <-time.After(testTimeout): - t.Fatal("timed out waiting for panic alert") - } - awaitTestLoggerQueue(t, tj.Jaws) - if got := tj.log.String(); !strings.Contains(got, detail) { - t.Fatalf("operator log omitted panic detail: %q", got) - } -} - -func TestRequest_HookHandlerPanicAlertHidesDetails(t *testing.T) { - rq := newTestRequest(t) - defer rq.Close() - ui := &testUi{} - rq.Register(ui, func(*Element, string) error { - panic("private panic detail") - }) - rq.Jaws.Broadcast(wire.Message{Dest: ui, What: what.Hook}) +func TestRequest_EventErrorAlerts(t *testing.T) { + for _, tt := range []struct { + name string + panic bool + hook bool + alert bool + logOnly bool + }{ + {name: "returned error"}, + {name: "panic", panic: true}, + {name: "hook returned error", hook: true}, + {name: "hook panic", hook: true, panic: true}, + {name: "explicit alert before panic", panic: true, alert: true}, + {name: "wrapped log-only error", logOnly: true}, + {name: "hook wrapped log-only error", hook: true, logOnly: true}, + } { + t.Run(tt.name, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw, err := New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + logger := new(eventErrorLogger) + jw.Logger = logger + go jw.Serve() + rq := newWrappedTestRequest(jw, nil) + defer rq.Close() + <-rq.ReadyCh + + wantErr := errors.New("private handler detail") + const alert = "warning\nChoose a valid value." + ui := &testUi{} + id := rq.Register(ui, func(elem *Element, _ string) error { + if tt.alert { + elem.Request.Alert("warning", "Choose a valid value.") + } + if tt.panic { + panic(wantErr) + } + if tt.logOnly { + return fmt.Errorf("%w: %w", ErrEventLogOnly, wantErr) + } + return wantErr + }) + if tt.hook { + rq.Jaws.Broadcast(wire.Message{Dest: ui, What: what.Hook}) + } else { + rq.InCh <- wire.WsMsg{Jid: id, What: what.Input} + } + synctest.Wait() - select { - case msg := <-rq.OutCh: - if msg.What != what.Alert || msg.Data != "danger\nevent handler failed" { - t.Fatalf("hook panic alert = %#v, want generic danger alert", msg) - } - case <-time.After(testTimeout): - t.Fatal("timed out waiting for hook panic alert") + logged := logger.loggedErrors() + if len(logged) != 1 || !errors.Is(logged[0], wantErr) || errors.Is(logged[0], ErrEventHandlerPanic) != tt.panic { + t.Fatalf("logged errors = %v, want handler error with panic=%v", logged, tt.panic) + } + if rq.Context().Err() != nil { + t.Fatal("handler failure cancelled the Request") + } + if tt.alert || (!tt.panic && !tt.logOnly) { + want := wire.WsMsg{What: what.Alert, Data: alert} + if !tt.alert { + want.Data = "danger\n" + wantErr.Error() + if tt.hook { + want.Jid = id + } + } + select { + case msg := <-rq.OutCh: + if msg != want { + t.Fatalf("browser alert = %#v, want %#v", msg, want) + } + default: + t.Fatal("missing browser alert") + } + } + select { + case msg := <-rq.OutCh: + t.Fatalf("unexpected browser message: %#v", msg) + default: + } + }) + }) } } diff --git a/requestloop.go b/requestloop.go index 8bfd5d61..fb4e7182 100644 --- a/requestloop.go +++ b/requestloop.go @@ -187,10 +187,7 @@ func (rq *Request) handleBroadcast(tagmsg wire.Message, eventCallCh chan eventFn // [what.Hook]. They exist for testing: the JaWS client never sends Hook, // so they only arrive here via Broadcast. The handler must not send any // messages itself. Returned errors follow the event alert policy. - if err := rq.Jaws.Log(rq.callAllEventHandlers(elem.Jid(), tagmsg.What, tagmsg.Data)); err != nil { - if err = eventAlertError(err); err == nil { - continue - } + if err := rq.Jaws.Log(rq.callAllEventHandlers(elem.Jid(), tagmsg.What, tagmsg.Data)); err != nil && !errors.Is(err, ErrEventLogOnly) { var m wire.WsMsg m.FillAlert(err) m.Jid = elem.Jid() @@ -542,13 +539,9 @@ func (rq *Request) eventCaller(eventCallCh <-chan eventFnCall, outboundMsgCh cha continue default: } - if err := rq.Jaws.Log(call.invoke()); err != nil { - alertErr := eventAlertError(err) - if alertErr == nil { - continue - } + if err := rq.Jaws.Log(call.invoke()); err != nil && !errors.Is(err, ErrEventLogOnly) { var m wire.WsMsg - m.FillAlert(alertErr) + m.FillAlert(err) // This error alert is best-effort: unlike queueEvent, which cancels the // Request with ErrRequestOverloaded when its channel fills (dropping a // queued event could desync browser and backend state), a dropped alert @@ -564,24 +557,6 @@ func (rq *Request) eventCaller(eventCallCh <-chan eventFnCall, outboundMsgCh cha } } -// clientAlertError selects safe browser text for library errors. -// An empty message suppresses the automatic alert, leaving logging unchanged. -type clientAlertError interface { - JawsClientAlert() string -} - -// eventAlertError returns the browser-safe error, or nil to suppress the alert. -func eventAlertError(err error) error { - var safe clientAlertError - if errors.As(err, &safe) { - if msg := safe.JawsClientAlert(); msg != "" { - return errors.New(msg) - } - return nil - } - return err -} - // onConnect calls the [Request]'s [ConnectFn] if it is not nil, returning its // error or converting its panic to an error. Returns nil if [ConnectFn] is nil. func (rq *Request) onConnect() (err error) { From 614769b639470bc96bc768e1c4f5efa71e1f7a1e Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Wed, 7 Oct 2026 08:37:25 +0200 Subject: [PATCH 3/3] docs: clarify event error feedback for callers --- contracts.go | 12 ++++++------ doc/runtime.md | 14 ++++++-------- doc/ui/jsvar.md | 7 +++---- errors.go | 14 ++++++-------- event.go | 7 +++---- lib/ui/jsvar.go | 12 ++++++------ lib/what/what.go | 11 ++++------- 7 files changed, 34 insertions(+), 43 deletions(-) diff --git a/contracts.go b/contracts.go index 1bbf0fa0..ba6c9a7d 100644 --- a/contracts.go +++ b/contracts.go @@ -141,9 +141,9 @@ type ClickHandler interface { // is found it falls back to the event target's HTML id, so it is empty only // when the target has no id either. // - // [ErrEventUnhandled] tries the next handler. Other non-nil errors are logged - // and queued as danger alerts when possible during Request event processing. - // Errors matching [ErrEventLogOnly], including recovered panics, omit the alert. + // [ErrEventUnhandled] tries the next handler. Request event processing logs + // other errors and, unless they match [ErrEventLogOnly], queues them as danger + // alerts when possible. JawsClick(elem *Element, click Click) (err error) } @@ -159,9 +159,9 @@ type ContextMenuHandler interface { // Events that occur while the bundled client's WebSocket is not open are not // forwarded or replayed. // - // [ErrEventUnhandled] tries the next handler. Other non-nil errors are logged - // and queued as danger alerts when possible during Request event processing. - // Errors matching [ErrEventLogOnly], including recovered panics, omit the alert. + // [ErrEventUnhandled] tries the next handler. Request event processing logs + // other errors and, unless they match [ErrEventLogOnly], queues them as danger + // alerts when possible. JawsContextMenu(elem *Element, click Click) (err error) } diff --git a/doc/runtime.md b/doc/runtime.md index ba5817b8..197fb20a 100644 --- a/doc/runtime.md +++ b/doc/runtime.md @@ -61,14 +61,12 @@ page dot does not install them. See `ConnectFn` in Register event handlers during rendering, before the Element is frozen. Dispatch tries attached handlers in reverse registration order, then the -Element's UI. Return `jaws.ErrEventUnhandled` to try the next handler. Other -errors are logged and, when possible, displayed as browser danger alerts. -JavaScript store rejections are logged and corrected without automatic alerts; -the application's `ClientCheck` can send its own [feedback](ui/jsvar.md). -Recovered panics are also logged without automatic alerts. Errors matching -`jaws.ErrEventLogOnly` suppress the automatic alert, including wrapped or joined -errors; explicit `Request.Alert` calls are unaffected. -If no handler accepts an event, it is ignored without an error or alert. +Element's UI. Return `jaws.ErrEventUnhandled` to try the next handler; an event +with no accepting handler is ignored. Other errors are logged and, when possible, +displayed as browser danger alerts unless they match `jaws.ErrEventLogOnly`. +Recovered panics and JsVar store rejections match this sentinel. During browser +events, use `Request.Alert` for explicit feedback, including inside +[`ClientCheck`](ui/jsvar.md). Browser event handlers run serially within each Request, concurrently with update processing and handlers in other Requests. Synchronize shared application state diff --git a/doc/ui/jsvar.md b/doc/ui/jsvar.md index 0d421ae8..91500c5e 100644 --- a/doc/ui/jsvar.md +++ b/doc/ui/jsvar.md @@ -42,10 +42,9 @@ a struct zeroes all its fields, including unexported and `json:"-"` fields. An error or panic rolls the proposal back. The callback must only inspect tentative data: do not mutate or retain it, reacquire the lock, or call a store setter. -Rejected proposals are logged and corrected without an automatic browser alert. -When handling browser input, `ClientCheck` can call -`source.Request.Alert("warning", "Choose a valid value.")` before returning an -error. Recovered panics are also logged without automatic alerts. +Rejected proposals, including panicking checks, are logged and corrected without +automatic browser alerts. When handling browser input, `ClientCheck` can call +`source.Request.Alert("warning", "Choose a valid value.")` for user feedback. Every binding receives the same JSON value; use separate stores for data with different visibility. Proposal outcomes can depend on Go fields omitted from diff --git a/errors.go b/errors.go index d8d9ed2f..bd7013ea 100644 --- a/errors.go +++ b/errors.go @@ -226,13 +226,11 @@ func newErrNoWebSocketRequest(rq *Request) error { return errNoWebSocketRequest{Addr: rq.remoteIP} } -// ErrEventHandlerPanic is returned by [CallEventHandlers] when a user event handler -// panics. +// ErrEventHandlerPanic reports a recovered event-handler panic. // -// Match it with [errors.Is]. When the recovered panic value is itself an error it is -// available via Unwrap (and thus [errors.As] / [errors.Is]); a non-error panic value -// appears only in the formatted message. Request event processing logs the full -// error without an automatic browser alert; it also matches [ErrEventLogOnly]. +// [CallEventHandlers] returns an error matching this sentinel and [ErrEventLogOnly]. +// It unwraps to the recovered value if that value is an error; other panic values +// appear only in the error text. var ErrEventHandlerPanic errEventHandlerPanic type errEventHandlerPanic struct { @@ -261,8 +259,8 @@ func (e errEventHandlerPanic) Unwrap() error { // ErrEventLogOnly marks event errors that are logged without automatic browser alerts. // // Request event processing suppresses an error's alert when [errors.Is] matches -// this sentinel, including wrapped or joined errors. Explicit [Request.Alert] -// calls are unaffected. +// this sentinel. Recovered handler panics and JsVar rejections match it. +// Explicit [Request.Alert] calls are unaffected. var ErrEventLogOnly = errors.New("event error is log-only") type errEventUnhandled struct{} diff --git a/event.go b/event.go index 9a362171..a420a976 100644 --- a/event.go +++ b/event.go @@ -145,10 +145,9 @@ type InputHandler interface { // its WebSocket is open and does not queue them for later delivery. Native // changes that emit neither input nor change do not invoke JawsInput. // - // [ErrEventUnhandled] tries the next handler. Other non-nil errors are logged - // and queued as danger alerts when possible during Request event processing. - // Errors matching [ErrEventLogOnly], including recovered panics and JsVar store - // rejections, are logged without automatic alerts. + // [ErrEventUnhandled] tries the next handler. Request event processing logs + // other errors and, unless they match [ErrEventLogOnly], queues them as danger + // alerts when possible. JawsInput(elem *Element, value string) (err error) } diff --git a/lib/ui/jsvar.go b/lib/ui/jsvar.go index b8e87c2c..e6ff8ec2 100644 --- a/lib/ui/jsvar.go +++ b/lib/ui/jsvar.go @@ -82,9 +82,10 @@ func validateJsVarPath(path string) error { // JsVarCheck validates the complete tentative state of a browser proposal. // // A nil check denies browser writes. The check runs under the store's write lock -// after jq tentatively applies a changed Go value. It must only inspect next; -// an error or panic rolls the proposal back. The source can authorize a user -// or session. Validate the complete value, including fields omitted from JSON +// after jq tentatively applies a changed Go value. It must not mutate or retain +// next, reacquire the lock, or call a store setter. An error or panic rolls the +// proposal back. The source can authorize a user or session. +// Validate the complete value, including fields omitted from JSON // and changes through parent or root paths. A null proposal for a struct zeroes // all its fields, including unexported and json:"-" fields. The check sees // converted Go values, not raw browser JSON; see [JsVarBinding.JawsInput]. @@ -591,10 +592,9 @@ func (store *JsVarStore[T]) projectVisiblePatch(path, root string, visible any) // A rejected, invalid, or unchanged proposal schedules a canonical correction // for its source binding. A changed accepted proposal invalidates every binding. // An unchanged proposal to a complex Go shape is rejected. -// Returned errors match [jaws.ErrEventLogOnly] and are logged during event -// processing without automatic alerts. +// Returned errors match [jaws.ErrEventLogOnly]. // A panicking check rolls back and schedules a root correction before the panic -// continues. [ErrJsVarTooLarge] cancels the source Request for reload recovery. +// continues. [ErrJsVarTooLarge] cancels the source Request. // // Browser JSON numbers are decoded as float64 and converted by jq to the Go // destination before ClientCheck runs. Conversion may lose information: 300 diff --git a/lib/what/what.go b/lib/what/what.go index 76d1caed..22b9e784 100644 --- a/lib/what/what.go +++ b/lib/what/what.go @@ -69,13 +69,10 @@ const ( // Hook synchronously invokes the matching event handler. // - // Hook is a testing facility rather than part of the browser wire protocol: - // the JaWS client never sends it, and inbound client messages are never - // dispatched as Hook. Broadcasting a Hook message lets a test drive an - // element's event handler synchronously, without round-tripping through the - // client. The handler must not send messages of its own. Returned errors are - // logged and normally sent as [Alert] messages; recovered panics and JsVar store - // rejections omit alerts. + // Hook is a test-only broadcast event; inbound client messages never dispatch + // it. Handlers must not send messages of their own. Errors are logged and + // normally sent as [Alert] messages; recovered panics and JsVar store rejections + // omit alerts. Hook )