Skip to content

Commit 596eced

Browse files
authored
Documentation-only improvements from the code-quality review (#81)
No behavior change; comments only. Build, go vet, staticcheck, golangci-lint and the full -race / -tags debug -race suites are unaffected. - Element queue helpers (SetAttr/SetInner/...): replace the misleading "call only during JawsRender/JawsUpdate" line with the accurate deferred-delivery contract and point to Request.Dirty for the event-driven path; expand the Element.queue chokepoint doc likewise. - contracts.go: fix the dangling [Element.Context]/[Element.Session] doc-links to [Request.Context]/[Request.Session]; give Container.JawsContains a leading synopsis sentence; note the deleted-Element no-op on Renderer.JawsRender and Updater.JawsUpdate. - eventhandler.go: document ErrEventHandlerPanic's Unwrap semantics and that the recorded Type is the UI object type, which may differ from the handler that panicked. - request.go: split Request.Get/Set synopses and add [Session.Get]/[Session.Set] links; document that AlertError also logs; note that Request methods are not nil-safe (unlike Session); record the recycle-window reasoning on appendDirtyTags and the retained-back-pointer/frozen invariant in clearLocked. - broadcast.go: document distributeDirt's deliberate pending-buffer and recycle-window behavior. - jaws.go / session.go: note the CSPRNG panic condition on NewRequest/NewSession. - errvalueunchanged.go: name the JawsSet/JawsSetPath convention that returns it and why the sentinel lives in this package. - element.go: link the public [CallEventHandlers] entry from the appendHandlers note.
1 parent 9ef8c00 commit 596eced

8 files changed

Lines changed: 124 additions & 29 deletions

File tree

‎broadcast.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,22 @@ func sortedDirtTags(dirty map[any]int) []any {
139139
return dirt
140140
}
141141

142+
// distributeDirt drains the accumulated dirty tags and appends them to every live
143+
// Request's pending-dirt list for the next update pass.
144+
//
145+
// It snapshots the Request set under jw.mu, releases it, then appends to each Request
146+
// without holding jw.mu. Two consequences are deliberate and harmless:
147+
//
148+
// - The snapshot includes pending (not-yet-running) Requests whose process loop has
149+
// not started, so their todoDirt buffers until they connect or are recycled
150+
// (bounded by the request timeout). This is intentional: a value mutated in the
151+
// render-to-connect window is then reflected on the first update pass.
152+
// - A Request can be recycled and reused for a different connection between the
153+
// snapshot and the append. appendDirtyTags and clearLocked both take rq.mu, so
154+
// there is no data race, and stale tags landing in a reborn Request resolve to
155+
// nothing in [Request.makeUpdateList] against its freshly emptied tagMap (at worst
156+
// a redundant re-render). Unlike destKey/cancelIfCurrent this path needs no
157+
// key-identity guard because applying dirt is idempotent.
142158
func (jw *Jaws) distributeDirt() int {
143159
var reqs []*Request
144160
var dirt []any

‎contracts.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,19 @@ import (
99
// Container is implemented by UI values that render a dynamic list of child
1010
// [UI] values.
1111
type Container interface {
12-
// JawsContains must return a slice of comparable [UI] objects (they are used
13-
// as map keys; see [UI] for the comparability requirement). The slice contents
14-
// must not be modified after returning it.
12+
// JawsContains returns the current child [UI] values contained by elem.
13+
//
14+
// The returned [UI] values must be comparable, since they are used as map keys
15+
// (see [UI] for the comparability requirement), and the slice contents must not
16+
// be modified after returning it.
1517
JawsContains(elem *Element) (contents []UI)
1618
}
1719

1820
// InitHandler allows initializing UI getters and setters before their use.
1921
//
2022
// You can of course initialize them in the call from the template engine,
21-
// but at that point you don't have access to the [Element], [Element.Context]
22-
// or [Element.Session].
23+
// but at that point you don't have access to the [Element], [Request.Context]
24+
// or [Request.Session].
2325
type InitHandler interface {
2426
JawsInit(elem *Element) (err error)
2527
}
@@ -35,6 +37,7 @@ type Logger interface {
3537
type Renderer interface {
3638
// JawsRender is called once per [Element] when rendering the initial webpage.
3739
// Do not call this yourself unless it is from within another JawsRender implementation.
40+
// The engine does not invoke this once the [Element] is deleted (see [Element.Deleted]).
3841
JawsRender(elem *Element, w io.Writer, params []any) error
3942
}
4043

@@ -62,6 +65,7 @@ type UI interface {
6265
type Updater interface {
6366
// JawsUpdate is called for an [Element] that has been marked dirty to update its HTML.
6467
// Do not call this yourself unless it is from within another JawsUpdate implementation.
68+
// The engine does not invoke this once the [Element] is deleted (see [Element.Deleted]).
6569
JawsUpdate(elem *Element)
6670
}
6771

‎element.go‎

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ func (elem *Element) String() string {
4444

4545
// appendHandlers is the single internal chokepoint for mutating elem.handlers.
4646
//
47-
// handlers is read lock-free on the event goroutine (see callEventHandlers), so
48-
// it must only be appended to while the Element is being rendered, before any
47+
// handlers is read lock-free on the event goroutine (via [CallEventHandlers], which
48+
// calls the internal callEventHandlers), so it must only be appended to while the
49+
// Element is being rendered, before any
4950
// event for it can fire. Once frozen, late mutations are a bug: reportMisuse
5051
// panics in debug builds and logs in production, and the mutation is dropped
5152
// rather than racing the lock-free read.
@@ -160,7 +161,13 @@ func (elem *Element) JawsUpdate() {
160161

161162
// queue enqueues a wire message of the given type and data for this element on
162163
// its Request, tagged with the element's Jid. It is a no-op once the element has
163-
// been deleted. Call only during JawsRender or JawsUpdate processing.
164+
// been deleted.
165+
//
166+
// It is intended to be called while the element is rendering or updating; the
167+
// message is appended to the Request's muQueue-guarded outbound queue and flushed on
168+
// the next processing-loop pass. Calling it from an event handler is safe but defers
169+
// delivery to that pass, so the event-driven path is to mark the element dirty (see
170+
// [Request.Dirty]), which schedules a [Updater.JawsUpdate].
164171
func (elem *Element) queue(wht what.What, data string) {
165172
if !elem.deleted.Load() {
166173
elem.Request.queue(wire.WsMsg{
@@ -177,47 +184,65 @@ func (elem *Element) queue(wht what.What, data string) {
177184
// The value parameter must be the unescaped logical attribute value. It is sent
178185
// to the browser DOM and used as the value argument to setAttribute().
179186
//
180-
// Call this only during JawsRender() or JawsUpdate() processing.
187+
// Call this while the [Element] is rendering or updating. The change is queued and
188+
// sent on the next processing pass; to change the [Element] in response to a browser
189+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
190+
// handler only defers the change to the next pass.
181191
func (elem *Element) SetAttr(attr, value string) {
182192
elem.queue(what.SAttr, attr+"\n"+value)
183193
}
184194

185195
// RemoveAttr queues sending a request to remove an attribute
186196
// to the browser for the [Element].
187197
//
188-
// Call this only during JawsRender() or JawsUpdate() processing.
198+
// Call this while the [Element] is rendering or updating. The change is queued and
199+
// sent on the next processing pass; to change the [Element] in response to a browser
200+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
201+
// handler only defers the change to the next pass.
189202
func (elem *Element) RemoveAttr(attr string) {
190203
elem.queue(what.RAttr, attr)
191204
}
192205

193206
// SetClass queues sending a class
194207
// to the browser for the [Element].
195208
//
196-
// Call this only during JawsRender() or JawsUpdate() processing.
209+
// Call this while the [Element] is rendering or updating. The change is queued and
210+
// sent on the next processing pass; to change the [Element] in response to a browser
211+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
212+
// handler only defers the change to the next pass.
197213
func (elem *Element) SetClass(cls string) {
198214
elem.queue(what.SClass, cls)
199215
}
200216

201217
// RemoveClass queues sending a request to remove a class
202218
// to the browser for the [Element].
203219
//
204-
// Call this only during JawsRender() or JawsUpdate() processing.
220+
// Call this while the [Element] is rendering or updating. The change is queued and
221+
// sent on the next processing pass; to change the [Element] in response to a browser
222+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
223+
// handler only defers the change to the next pass.
205224
func (elem *Element) RemoveClass(cls string) {
206225
elem.queue(what.RClass, cls)
207226
}
208227

209228
// SetInner queues sending new inner HTML content
210229
// to the browser for the [Element].
211230
//
212-
// Call this only during JawsRender() or JawsUpdate() processing.
231+
// Call this while the [Element] is rendering or updating. The change is queued and
232+
// sent on the next processing pass; to change the [Element] in response to a browser
233+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
234+
// handler only defers the change to the next pass.
213235
func (elem *Element) SetInner(innerHTML template.HTML) {
214236
elem.queue(what.Inner, string(innerHTML))
215237
}
216238

217239
// SetValue queues sending a new current input value in textual form
218240
// to the browser for the [Element].
219241
//
220-
// Call this only during JawsRender() or JawsUpdate() processing.
242+
// Call this while the [Element] is rendering or updating. The change is queued and
243+
// sent on the next processing pass; to change the [Element] in response to a browser
244+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
245+
// handler only defers the change to the next pass.
221246
func (elem *Element) SetValue(value string) {
222247
elem.queue(what.Value, value)
223248
}
@@ -228,7 +253,10 @@ func (elem *Element) SetValue(value string) {
228253
// is a programming error: debug builds panic and production builds report it via
229254
// [Jaws.MustLog] and skip the replacement.
230255
//
231-
// Call this only during JawsRender() or JawsUpdate() processing.
256+
// Call this while the [Element] is rendering or updating. The change is queued and
257+
// sent on the next processing pass; to change the [Element] in response to a browser
258+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
259+
// handler only defers the change to the next pass.
232260
func (elem *Element) Replace(htmlCode template.HTML) {
233261
if !elem.deleted.Load() {
234262
var b []byte
@@ -246,14 +274,20 @@ func (elem *Element) Replace(htmlCode template.HTML) {
246274

247275
// Append appends a new HTML element as a child to the current one.
248276
//
249-
// Call this only during JawsRender() or JawsUpdate() processing.
277+
// Call this while the [Element] is rendering or updating. The change is queued and
278+
// sent on the next processing pass; to change the [Element] in response to a browser
279+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
280+
// handler only defers the change to the next pass.
250281
func (elem *Element) Append(htmlCode template.HTML) {
251282
elem.queue(what.Append, string(htmlCode))
252283
}
253284

254285
// Order reorders the HTML elements.
255286
//
256-
// Call this only during JawsRender() or JawsUpdate() processing.
287+
// Call this while the [Element] is rendering or updating. The change is queued and
288+
// sent on the next processing pass; to change the [Element] in response to a browser
289+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
290+
// handler only defers the change to the next pass.
257291
func (elem *Element) Order(jidList []jid.Jid) {
258292
if !elem.deleted.Load() && len(jidList) > 0 {
259293
var b []byte
@@ -270,7 +304,10 @@ func (elem *Element) Order(jidList []jid.Jid) {
270304
// Remove requests that the HTML child with the given HTML ID of this [Element]
271305
// is removed from the [Request] and its HTML element from the browser.
272306
//
273-
// Call this only during JawsRender() or JawsUpdate() processing.
307+
// Call this while the [Element] is rendering or updating. The change is queued and
308+
// sent on the next processing pass; to change the [Element] in response to a browser
309+
// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
310+
// handler only defers the change to the next pass.
274311
func (elem *Element) Remove(htmlID string) {
275312
elem.queue(what.Remove, htmlID)
276313
}

‎errvalueunchanged.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ package jaws
22

33
import "errors"
44

5-
// ErrValueUnchanged can be returned from JawsSet methods
6-
// to indicate that while there was no error, the underlying value
7-
// was already the desired value.
5+
// ErrValueUnchanged reports a successful no-op set: there was no error, but the
6+
// underlying value already equaled the desired value.
7+
//
8+
// Setter-style implementations (the JawsSet / JawsSetPath methods in
9+
// github.com/linkdata/jaws/lib/ui and github.com/linkdata/jaws/jawstree) return it,
10+
// and callers test for it with [errors.Is]. It lives in this package so all
11+
// implementations share one error identity.
812
var ErrValueUnchanged = errors.New("value unchanged")

‎eventhandler.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,20 @@ import (
88
"github.com/linkdata/jaws/lib/what"
99
)
1010

11-
// ErrEventHandlerPanic is returned when an event handler panics.
11+
// ErrEventHandlerPanic is returned by [CallEventHandlers] when a user event handler
12+
// panics.
13+
//
14+
// Match it with [errors.Is]. When the recovered panic value is itself an error it is
15+
// available via Unwrap (and thus [errors.As] / [errors.Is]); a non-error panic value
16+
// appears only in the formatted message.
1217
var ErrEventHandlerPanic errEventHandlerPanic
1318

1419
type errEventHandlerPanic struct {
20+
// Type is the [Element]'s UI object type. Handlers registered on the Element are
21+
// tried before the UI object, so the type that actually panicked may differ from
22+
// this when a registered handler is the culprit.
1523
Type reflect.Type
16-
Value any
24+
Value any // the recovered panic value
1725
}
1826

1927
func (e errEventHandlerPanic) Error() string {

‎jaws.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ func MakeID() string {
378378
//
379379
// Automatic timeout handling is performed by [Jaws.ServeWithTimeout]. The default
380380
// [Jaws.Serve] helper uses a 10-second timeout.
381+
//
382+
// It panics if the system CSPRNG ([crypto/rand]) fails while generating the request
383+
// key, which does not happen on supported platforms.
381384
func (jw *Jaws) NewRequest(r *http.Request) (rq *Request) {
382385
remoteIP := jw.clientIP(r)
383386

‎request.go‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ type ConnectFn = func(rq *Request) error
4242
//
4343
// Note that we have to store the context inside the struct because there is no call chain
4444
// between the Request being created and it being used once the WebSocket is created.
45+
//
46+
// Unlike [Session], whose methods are nil-safe, Request methods are not safe to call on a
47+
// nil *Request: a Request is always obtained from [Jaws.NewRequest] or [Jaws.UseRequest]
48+
// and is never legitimately nil. The nil-receiver guards on [Request.Log] and
49+
// [Request.MustLog] exist only so a nil Request can be rendered into error text, not as a
50+
// public nil-safe contract.
4551
type Request struct {
4652
Jaws *Jaws // (read-only) the JaWS instance the Request belongs to
4753
JawsKey key.Key // (read-only) random key identifying this Request in the WebSocket URI and the broadcast/tail target; read under mu by the identity check (destKey, wantMessage) and the render path (JawsKeyString, HeadHTML)
@@ -225,6 +231,12 @@ func (rq *Request) clearLocked() *Request {
225231
rq.remoteIP = netip.Addr{}
226232
for _, e := range rq.elems {
227233
if e != nil {
234+
// Nil the GC-reachable fields and set the deleted guard, which makes any
235+
// retained *Element inert (see [Element.Deleted]). The Request back-pointer
236+
// and frozen flag are deliberately left as-is: Elements are allocated fresh
237+
// per newElementLocked and never pooled, so a stale frozen value can never
238+
// be observed by a reused Element. Any future move to pool Elements must
239+
// also reset frozen, since it gates the lock-free handler read.
228240
e.handlers = nil
229241
e.ui = nil
230242
e.deleted.Store(true)
@@ -402,15 +414,18 @@ func (rq *Request) Initial() (r *http.Request) {
402414
return
403415
}
404416

405-
// Get is shorthand for Session().Get and returns the session value associated with the key, or nil.
406-
// If no session is associated with the [Request], it returns nil.
417+
// Get is shorthand for [Session.Get].
418+
//
419+
// It returns the session value associated with key, or nil if no session is associated
420+
// with the [Request].
407421
func (rq *Request) Get(key string) any {
408422
return rq.Session().Get(key)
409423
}
410424

411-
// Set is shorthand for Session().Set and sets a session value to be associated with the key.
412-
// If value is nil, the key is removed from the session.
413-
// Does nothing if no session is associated with the [Request].
425+
// Set is shorthand for [Session.Set].
426+
//
427+
// It associates value with key in the session; a nil value removes the key. It does
428+
// nothing if no session is associated with the [Request].
414429
func (rq *Request) Set(key string, value any) {
415430
rq.Session().Set(key, value)
416431
}
@@ -532,7 +547,8 @@ func (rq *Request) Alert(level, msg string) {
532547
}
533548
}
534549

535-
// AlertError calls [Request.Alert] if the given error is not nil.
550+
// AlertError logs err via [Jaws.Log] and, if it is non-nil, also shows it to the
551+
// current request as a danger-level [Request.Alert].
536552
func (rq *Request) AlertError(err error) {
537553
if rq.Jaws.Log(err) != nil {
538554
rq.Alert("danger", err.Error())
@@ -675,6 +691,10 @@ func (rq *Request) HasTag(elem *Element, tagValue any) (yes bool) {
675691
// appendDirtyTags queues already-expanded tags onto this request's pending-dirt
676692
// list. The Serve loop's update tick later drains the list (see makeUpdateList)
677693
// and re-renders the affected elements. Takes rq.mu.
694+
//
695+
// It may run on a Request that was recycled and reused after the caller's dirt
696+
// snapshot was taken; that is race-free (clearLocked also takes rq.mu) and harmless,
697+
// as explained on distributeDirt.
678698
func (rq *Request) appendDirtyTags(tags []any) {
679699
rq.mu.Lock()
680700
rq.todoDirt = append(rq.todoDirt, tags...)

‎session.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,9 @@ func (jw *Jaws) GetSession(r *http.Request) (sess *Session) {
344344
// As a side effect, the session cookie is also added to r itself, so the new
345345
// [Session] is visible to [Jaws.GetSession] and [Jaws.NewRequest] for the
346346
// remainder of the same HTTP request.
347+
//
348+
// It panics if the system CSPRNG ([crypto/rand]) fails while generating the session
349+
// ID, which does not happen on supported platforms.
347350
func (jw *Jaws) NewSession(w http.ResponseWriter, r *http.Request) (sess *Session) {
348351
if r != nil {
349352
if oldSess := jw.GetSession(r); oldSess != nil {

0 commit comments

Comments
 (0)