Skip to content

Commit 06ecdab

Browse files
Copilotintel352
andauthored
Fix silent error suppression across workflow engine (#114)
* Initial plan * Fix silent error suppression: log errors in jwt_auth, persistence ops, distributed lock, integration, http handlers Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Co-authored-by: Jonathan Langevin <codingsloth@pm.me>
1 parent 83f26ed commit 06ecdab

File tree

8 files changed

+29
-18
lines changed

8 files changed

+29
-18
lines changed

module/api_crud_handler.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,9 @@ func (h *RESTAPIHandler) handlePost(resourceId string, w http.ResponseWriter, r
196196
if h.persistence != nil {
197197
h.fieldMapping.SetValue(resource.Data, "state", resource.State)
198198
h.fieldMapping.SetValue(resource.Data, "lastUpdate", resource.LastUpdate)
199-
_ = h.persistence.SaveResource(h.resourceName, resource.ID, resource.Data)
199+
if err := h.persistence.SaveResource(h.resourceName, resource.ID, resource.Data); err != nil {
200+
h.logger.Warn(fmt.Sprintf("failed to persist resource %s/%s: %v", h.resourceName, resource.ID, err))
201+
}
200202
}
201203

202204
// If a workflow engine is configured, create an instance and trigger the initial transition
@@ -252,7 +254,9 @@ func (h *RESTAPIHandler) handlePut(resourceId string, w http.ResponseWriter, r *
252254

253255
// Write-through to persistence
254256
if h.persistence != nil {
255-
_ = h.persistence.SaveResource(h.resourceName, resourceId, data)
257+
if err := h.persistence.SaveResource(h.resourceName, resourceId, data); err != nil {
258+
h.logger.Warn(fmt.Sprintf("failed to persist resource %s/%s: %v", h.resourceName, resourceId, err))
259+
}
256260
}
257261

258262
if err := json.NewEncoder(w).Encode(h.resources[resourceId]); err != nil {
@@ -287,7 +291,9 @@ func (h *RESTAPIHandler) handleDelete(resourceId string, w http.ResponseWriter,
287291

288292
// Write-through to persistence
289293
if h.persistence != nil {
290-
_ = h.persistence.DeleteResource(h.resourceName, resourceId)
294+
if err := h.persistence.DeleteResource(h.resourceName, resourceId); err != nil {
295+
h.logger.Warn(fmt.Sprintf("failed to delete persisted resource %s/%s: %v", h.resourceName, resourceId, err))
296+
}
291297
}
292298

293299
w.WriteHeader(http.StatusNoContent)

module/api_v1_handler.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package module
33
import (
44
"encoding/json"
55
"fmt"
6+
"log"
67
"net/http"
78
"os"
89
"path/filepath"
@@ -917,7 +918,7 @@ func (h *V1APIHandler) stopWorkflow(w http.ResponseWriter, r *http.Request, id s
917918
if inst, ok := h.runtimeManager.GetInstance(id); ok && inst.Status == "running" {
918919
if stopErr := h.runtimeManager.StopWorkflow(r.Context(), id); stopErr != nil {
919920
// Log but don't fail — the DB status update should still proceed
920-
_ = stopErr
921+
log.Printf("workflow engine: failed to stop workflow %s: %v", id, stopErr)
921922
}
922923
}
923924
}

module/api_workflow_handler.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,9 @@ func (h *RESTAPIHandler) syncResourceStateFromEngine(instanceId, resourceId stri
108108

109109
// Write-through to persistence
110110
if h.persistence != nil {
111-
_ = h.persistence.SaveResource(h.resourceName, res.ID, res.Data)
111+
if err := h.persistence.SaveResource(h.resourceName, res.ID, res.Data); err != nil {
112+
h.logger.Warn(fmt.Sprintf("failed to persist resource %s/%s: %v", h.resourceName, res.ID, err))
113+
}
112114
}
113115
}
114116
}

module/http_handlers.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package module
22

33
import (
44
"encoding/json"
5+
"log"
56
"net/http"
67

78
"github.com/CrisisTextLine/modular"
@@ -54,8 +55,7 @@ func (h *SimpleHTTPHandler) Handle(w http.ResponseWriter, r *http.Request) {
5455
}
5556

5657
if err := json.NewEncoder(w).Encode(response); err != nil {
57-
// Log error but continue since response is already committed
58-
_ = err
58+
log.Printf("http handler: failed to encode response: %v", err)
5959
}
6060
}
6161

module/http_trigger.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package module
33
import (
44
"context"
55
"fmt"
6+
"log"
67
"maps"
78
"net/http"
89

@@ -246,9 +247,7 @@ func (t *HTTPTrigger) createHandler(route HTTPTriggerRoute) HTTPHandler {
246247
w.Header().Set("Content-Type", "application/json")
247248
w.WriteHeader(http.StatusAccepted)
248249
if _, err := w.Write([]byte(`{"status": "workflow triggered"}`)); err != nil {
249-
// Log error but don't fail the trigger
250-
// Note: In a real implementation, we'd need access to a logger here
251-
_ = err // Explicitly ignore error to satisfy linter
250+
log.Printf("http trigger: failed to write response: %v", err)
252251
}
253252
}
254253

module/integration.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"io"
8+
"log"
89
"net"
910
"net/http"
1011
"net/url"
@@ -292,8 +293,7 @@ func (c *HTTPIntegrationConnector) Execute(ctx context.Context, action string, p
292293
}
293294
defer func() {
294295
if err := resp.Body.Close(); err != nil {
295-
// Log error but continue
296-
_ = err // Explicitly ignore error to satisfy linter
296+
log.Printf("integration: failed to close response body: %v", err)
297297
}
298298
}()
299299

@@ -372,8 +372,7 @@ func (c *WebhookIntegrationConnector) Connect(ctx context.Context) error {
372372
// Parse the request body
373373
defer func() {
374374
if err := r.Body.Close(); err != nil {
375-
// Log error but continue
376-
_ = err // Explicitly ignore error to satisfy linter
375+
log.Printf("integration: failed to close request body: %v", err)
377376
}
378377
}()
379378
body, err := io.ReadAll(r.Body)
@@ -414,8 +413,7 @@ func (c *WebhookIntegrationConnector) Connect(ctx context.Context) error {
414413
// Return success
415414
w.WriteHeader(http.StatusOK)
416415
if _, err := w.Write([]byte(`{"status":"ok"}`)); err != nil {
417-
// Log error but continue
418-
_ = err // Explicitly ignore error to satisfy linter
416+
log.Printf("integration: failed to write webhook response: %v", err)
419417
}
420418
})
421419

module/jwt_auth.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ type JWTAuthModule struct {
4040
mu sync.RWMutex
4141
nextID int
4242
app modular.Application
43+
logger modular.Logger
4344
persistence *PersistenceStore // optional write-through backend
4445
userStore *UserStore // optional external user store (from auth.user-store module)
4546
}
@@ -89,6 +90,7 @@ func (j *JWTAuthModule) Init(app modular.Application) error {
8990
return fmt.Errorf("JWT secret must be at least 32 bytes for security")
9091
}
9192
j.app = app
93+
j.logger = app.Logger()
9294

9395
// Wire external user store (optional — from auth.user-store module)
9496
for _, svc := range app.SvcRegistry() {
@@ -1164,7 +1166,7 @@ func (j *JWTAuthModule) Start(ctx context.Context) error {
11641166
if j.seedFile != "" {
11651167
if err := j.loadSeedUsers(j.seedFile); err != nil {
11661168
// Non-fatal: log but don't prevent startup
1167-
_ = err
1169+
j.logger.Warn("failed to load seed users", "file", j.seedFile, "error", err)
11681170
}
11691171
}
11701172

scale/distributed_lock.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"encoding/hex"
88
"fmt"
99
"hash/fnv"
10+
"log"
1011
"sync"
1112
"time"
1213

@@ -311,7 +312,9 @@ func (l *RedisLock) buildRelease(key, token string) func() {
311312
return func() {
312313
once.Do(func() {
313314
ctx := context.Background()
314-
_ = redisReleaseScript.Run(ctx, l.client, []string{key}, token).Err()
315+
if err := redisReleaseScript.Run(ctx, l.client, []string{key}, token).Err(); err != nil {
316+
log.Printf("distributed lock: failed to release Redis lock for key %s: %v", key, err)
317+
}
315318
})
316319
}
317320
}

0 commit comments

Comments
 (0)