fix: API quality fixes from code review (HAN-398) #17

Merged
multica-agent merged 1 commits from fix/han-398-api-quality into main 2026-06-07 18:52:20 +00:00
2 changed files with 43 additions and 19 deletions
+27 -10
View File
@@ -86,10 +86,8 @@ func (j *TraversalJob) subscribe() <-chan ProgressEvent {
return ch
}
// publish sends an event to all current subscribers.
func (j *TraversalJob) publish(ev ProgressEvent) {
j.mu.Lock()
defer j.mu.Unlock()
// publishLocked sends ev to all current subscribers. Caller must hold j.mu.
func (j *TraversalJob) publishLocked(ev ProgressEvent) {
for _, ch := range j.subs {
select {
case ch <- ev:
@@ -99,6 +97,18 @@ func (j *TraversalJob) publish(ev ProgressEvent) {
}
}
// unsubscribe removes ch from the subscriber list.
func (j *TraversalJob) unsubscribe(ch <-chan ProgressEvent) {
j.mu.Lock()
defer j.mu.Unlock()
for i, s := range j.subs {
if s == ch {
j.subs = append(j.subs[:i], j.subs[i+1:]...)
return
}
}
}
// closeSubscribers drains and closes all subscriber channels.
func (j *TraversalJob) closeSubscribers() {
j.mu.Lock()
@@ -153,7 +163,7 @@ type Handler struct {
mux *http.ServeMux
}
func newHandler() *Handler {
func newHandler(ctx context.Context) *Handler {
h := &Handler{
st: newStore(),
mux: http.NewServeMux(),
@@ -164,13 +174,18 @@ func newHandler() *Handler {
h.mux.HandleFunc("GET /api/traverse/{id}", h.getTraversal)
h.mux.HandleFunc("GET /api/health", h.health)
// periodic cleanup
// periodic cleanup; exits when ctx is cancelled (e.g. on Server.Shutdown).
go func() {
t := time.NewTicker(10 * time.Minute)
defer t.Stop()
for range t.C {
for {
select {
case <-ctx.Done():
return
case <-t.C:
h.st.cleanup()
}
}
}()
return h
@@ -189,6 +204,7 @@ func (h *Handler) health(w http.ResponseWriter, _ *http.Request) {
// startTraversal handles POST /api/traverse.
func (h *Handler) startTraversal(w http.ResponseWriter, r *http.Request) {
r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1 MB limit
var req TraverseRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
writeError(w, http.StatusBadRequest, "invalid request body: "+err.Error())
@@ -285,8 +301,10 @@ func (h *Handler) streamTraversal(w http.ResponseWriter, r *http.Request) {
}
// Subscribe before snapshotting progress so we don't miss events between
// the two operations.
// the two operations. Unsubscribe when the client disconnects so stale
// channels don't accumulate.
sub := job.subscribe()
defer job.unsubscribe(sub)
// Replay events already recorded.
job.mu.RLock()
@@ -381,9 +399,8 @@ func (h *Handler) runTraversal(ctx context.Context, job *TraversalJob, domain st
job.mu.Lock()
job.Progress = append(job.Progress, ev)
job.publishLocked(ev) // inside lock: no race with subscribe+replay
job.mu.Unlock()
job.publish(ev)
},
}
+8 -1
View File
@@ -28,6 +28,7 @@ var staticFiles embed.FS
type Server struct {
addr string
srv *http.Server
cancel context.CancelFunc
}
// NewServer creates a new Server that listens on addr (e.g. ":8080").
@@ -39,7 +40,9 @@ func NewServer(addr string) *Server {
// server has accepted its first connection or the address is bound.
// Call Shutdown to stop gracefully.
func (s *Server) Start() error {
h := newHandler()
ctx, cancel := context.WithCancel(context.Background())
s.cancel = cancel
h := newHandler(ctx)
sub, err := fs.Sub(staticFiles, "static")
if err != nil {
@@ -50,6 +53,7 @@ func (s *Server) Start() error {
s.srv = &http.Server{
Addr: s.addr,
Handler: corsMiddleware(h.mux),
ReadHeaderTimeout: 10 * time.Second,
ReadTimeout: 30 * time.Second,
WriteTimeout: 0, // SSE streams need no write timeout
IdleTimeout: 120 * time.Second,
@@ -78,6 +82,9 @@ func (s *Server) Addr() string {
// Shutdown gracefully stops the server, waiting up to timeout for in-flight
// requests to complete.
func (s *Server) Shutdown(timeout time.Duration) error {
if s.cancel != nil {
s.cancel()
}
if s.srv == nil {
return nil
}