Skip to content

Commit fe576ef

Browse files
roychyingJamyDev
authored andcommitted
fix(controller): drop redundant ControllerName prefix from wrapped error messages (#420)
Remove redundant controllerName prefix in error msg body
1 parent b342ced commit fe576ef

9 files changed

Lines changed: 68 additions & 69 deletions

File tree

stovepipe/controller/build/build.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
107107
buildRunner, err := c.buildRunners.For(buildrunner.Config{QueueName: request.Queue})
108108
if err != nil {
109109
// A queue with no registered builder is a config error.
110-
return fmt.Errorf("BuildController failed to resolve build runner for queue %s: %w", request.Queue, err)
110+
return fmt.Errorf("failed to resolve build runner for queue %s: %w", request.Queue, err)
111111
}
112112

113113
// process decided the scope; build never re-derives incremental-vs-full.
@@ -122,7 +122,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
122122

123123
buildID, err := buildRunner.Trigger(ctx, baseURI, request.URI, nil)
124124
if err != nil {
125-
return fmt.Errorf("BuildController failed to trigger build for request %s: %w", request.ID, err)
125+
return fmt.Errorf("failed to trigger build for request %s: %w", request.ID, err)
126126
}
127127

128128
build := entity.Build{
@@ -132,11 +132,11 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
132132
Version: 1,
133133
}
134134
if err := c.store.GetBuildStore().Create(ctx, build); err != nil && !errors.Is(err, storage.ErrAlreadyExists) {
135-
return fmt.Errorf("BuildController failed to persist build %s: %w", build.ID, err)
135+
return fmt.Errorf("failed to persist build %s: %w", build.ID, err)
136136
}
137137

138138
if err := c.publishBuildSignal(ctx, build.ID); err != nil {
139-
return fmt.Errorf("BuildController failed to publish build signal for %s: %w", build.ID, err)
139+
return fmt.Errorf("failed to publish build signal for %s: %w", build.ID, err)
140140
}
141141

142142
c.logger.Debugw("triggered build",
@@ -150,7 +150,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
150150

151151
// loadRequest returns the request for id.
152152
func (c *Controller) loadRequest(ctx context.Context, id string) (entity.Request, error) {
153-
return loader.ByID(ctx, id, c.store.GetRequestStore().Get, "BuildController", "request")
153+
return loader.ByID(ctx, id, c.store.GetRequestStore().Get, "request")
154154
}
155155

156156
// publishBuildSignal publishes buildID to the buildsignal stage, partitioned by

stovepipe/controller/buildsignal/buildsignal.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,12 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
129129
buildRunner, err := c.buildRunners.For(buildrunner.Config{QueueName: request.Queue})
130130
if err != nil {
131131
// A queue with no registered builder is a config error.
132-
return fmt.Errorf("BuildSignalController failed to resolve build runner for queue %s: %w", request.Queue, err)
132+
return fmt.Errorf("failed to resolve build runner for queue %s: %w", request.Queue, err)
133133
}
134134

135135
status, _, err := buildRunner.Status(ctx, entity.BuildID{ID: build.ID})
136136
if err != nil {
137-
return fmt.Errorf("BuildSignalController failed to poll status for build %s: %w", build.ID, err)
137+
return fmt.Errorf("failed to poll status for build %s: %w", build.ID, err)
138138
}
139139

140140
effective, err := c.reconcile(ctx, build, status)
@@ -144,7 +144,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
144144

145145
if effective.IsTerminal() {
146146
if err := c.publishRecord(ctx, build.ID, request.ID); err != nil {
147-
return fmt.Errorf("BuildSignalController failed to publish record for build %s: %w", build.ID, err)
147+
return fmt.Errorf("failed to publish record for build %s: %w", build.ID, err)
148148
}
149149
c.logger.Infow("build reached terminal status",
150150
"build_id", build.ID,
@@ -156,7 +156,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
156156

157157
delayMs := pollDelay(effective)
158158
if err := c.publishBuildSignal(ctx, build.ID, delayMs); err != nil {
159-
return errs.NewRetryableError(fmt.Errorf("BuildSignalController failed to reschedule poll for build %s: %w", build.ID, err))
159+
return errs.NewRetryableError(fmt.Errorf("failed to reschedule poll for build %s: %w", build.ID, err))
160160
}
161161
c.logger.Debugw("rescheduled build status poll",
162162
"build_id", build.ID,
@@ -188,19 +188,19 @@ func (c *Controller) reconcile(ctx context.Context, build entity.Build, status e
188188
if errors.Is(err, storage.ErrVersionMismatch) {
189189
return "", errs.NewRetryableError(fmt.Errorf("build %s version conflict: %w", build.ID, err))
190190
}
191-
return "", fmt.Errorf("BuildSignalController failed to persist status for build %s: %w", build.ID, err)
191+
return "", fmt.Errorf("failed to persist status for build %s: %w", build.ID, err)
192192
}
193193
return status, nil
194194
}
195195

196196
// loadBuild returns the build for id.
197197
func (c *Controller) loadBuild(ctx context.Context, id string) (entity.Build, error) {
198-
return loader.ByID(ctx, id, c.store.GetBuildStore().Get, "BuildSignalController", "build")
198+
return loader.ByID(ctx, id, c.store.GetBuildStore().Get, "build")
199199
}
200200

201201
// loadRequest returns the request for id.
202202
func (c *Controller) loadRequest(ctx context.Context, id string) (entity.Request, error) {
203-
return loader.ByID(ctx, id, c.store.GetRequestStore().Get, "BuildSignalController", "request")
203+
return loader.ByID(ctx, id, c.store.GetRequestStore().Get, "request")
204204
}
205205

206206
// pollDelay returns the delay before the next Status call for a non-terminal status.

stovepipe/controller/ingest.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -93,22 +93,22 @@ func (c *IngestController) Ingest(ctx context.Context, req entity.IngestRequest)
9393
defer func() { op.Complete(retErr) }()
9494

9595
if req.Queue == "" {
96-
return entity.IngestResult{}, fmt.Errorf("IngestController requires the request to have a queue name specified: %w", ErrInvalidRequest)
96+
return entity.IngestResult{}, fmt.Errorf("requires the request to have a queue name specified: %w", ErrInvalidRequest)
9797
}
9898
queue := req.Queue
9999

100100
// Resolve the queue's current head commit to its opaque URI via SourceControl.
101101
// An unresolvable queue/ref is a caller error (unknown queue), not infrastructure.
102102
sc, err := c.sourceControl.For(sourcecontrol.Config{QueueName: queue})
103103
if err != nil {
104-
return entity.IngestResult{}, fmt.Errorf("IngestController failed to resolve source control for queue=%s: %w", queue, err)
104+
return entity.IngestResult{}, fmt.Errorf("failed to resolve source control for queue=%s: %w", queue, err)
105105
}
106106
uri, err := sc.Latest(ctx)
107107
if err != nil {
108108
if sourcecontrol.IsNotFound(err) {
109-
return entity.IngestResult{}, fmt.Errorf("IngestController could not resolve head for queue=%s: %w: %w", queue, err, ErrInvalidRequest)
109+
return entity.IngestResult{}, fmt.Errorf("could not resolve head for queue=%s: %w: %w", queue, err, ErrInvalidRequest)
110110
}
111-
return entity.IngestResult{}, fmt.Errorf("IngestController failed to resolve head for queue=%s: %w", queue, err)
111+
return entity.IngestResult{}, fmt.Errorf("failed to resolve head for queue=%s: %w", queue, err)
112112
}
113113

114114
// The (queue, URI) mapping is the dedup gate and the source of truth for "does this head
@@ -135,7 +135,7 @@ func (c *IngestController) Ingest(ctx context.Context, req entity.IngestRequest)
135135
// process advances the request past Accepted, ingest stops re-publishing.
136136
if request.State == entity.RequestStateAccepted {
137137
if err := c.publishProcess(ctx, id, queue); err != nil {
138-
return entity.IngestResult{}, fmt.Errorf("IngestController failed to publish request %s to process: %w", id, err)
138+
return entity.IngestResult{}, fmt.Errorf("failed to publish request %s to process: %w", id, err)
139139
}
140140
}
141141

@@ -159,27 +159,27 @@ func (c *IngestController) resolveID(ctx context.Context, queue, uri string) (st
159159
if id, err := uriStore.GetIDByURI(ctx, queue, uri); err == nil {
160160
return id, nil
161161
} else if !errors.Is(err, storage.ErrNotFound) {
162-
return "", fmt.Errorf("IngestController failed to look up existing request for queue=%s: %w", queue, err)
162+
return "", fmt.Errorf("failed to look up existing request for queue=%s: %w", queue, err)
163163
}
164164

165165
// Mint a globally unique request ID namespaced by the queue. The counter domain
166166
// ("request/<queue>") doubles as the ID prefix, so the ID is "<domain>/<counter>".
167167
domain := "request/" + queue
168168
seq, err := c.counter.Next(ctx, domain)
169169
if err != nil {
170-
return "", fmt.Errorf("IngestController failed to generate request ID for queue=%s: %w", queue, err)
170+
return "", fmt.Errorf("failed to generate request ID for queue=%s: %w", queue, err)
171171
}
172172
id := fmt.Sprintf("%s/%d", domain, seq)
173173

174174
if err := uriStore.Create(ctx, queue, uri, id); err != nil {
175175
if errors.Is(err, storage.ErrAlreadyExists) {
176176
existing, getErr := uriStore.GetIDByURI(ctx, queue, uri)
177177
if getErr != nil {
178-
return "", fmt.Errorf("IngestController failed to resolve raced request for queue=%s: %w", queue, getErr)
178+
return "", fmt.Errorf("failed to resolve raced request for queue=%s: %w", queue, getErr)
179179
}
180180
return existing, nil
181181
}
182-
return "", fmt.Errorf("IngestController failed to map URI for queue=%s: %w", queue, err)
182+
return "", fmt.Errorf("failed to map URI for queue=%s: %w", queue, err)
183183
}
184184
return id, nil
185185
}
@@ -194,7 +194,7 @@ func (c *IngestController) ensureRequest(ctx context.Context, id, queue, uri str
194194
return got, nil
195195
}
196196
if !errors.Is(err, storage.ErrNotFound) {
197-
return entity.Request{}, fmt.Errorf("IngestController failed to load request %s: %w", id, err)
197+
return entity.Request{}, fmt.Errorf("failed to load request %s: %w", id, err)
198198
}
199199

200200
request := entity.Request{
@@ -206,7 +206,7 @@ func (c *IngestController) ensureRequest(ctx context.Context, id, queue, uri str
206206
}
207207
if err := reqStore.Create(ctx, request); err != nil {
208208
if !errors.Is(err, storage.ErrAlreadyExists) {
209-
return entity.Request{}, fmt.Errorf("IngestController failed to persist request %s: %w", id, err)
209+
return entity.Request{}, fmt.Errorf("failed to persist request %s: %w", id, err)
210210
}
211211
// Raced with a concurrent creator; read the canonical row.
212212
return reqStore.Get(ctx, id)
@@ -224,7 +224,7 @@ func (c *IngestController) ensureQueue(ctx context.Context, name string) (entity
224224
return got, nil
225225
}
226226
if !errors.Is(err, storage.ErrNotFound) {
227-
return entity.Queue{}, fmt.Errorf("IngestController failed to load queue %s: %w", name, err)
227+
return entity.Queue{}, fmt.Errorf("failed to load queue %s: %w", name, err)
228228
}
229229

230230
queue := entity.Queue{
@@ -233,7 +233,7 @@ func (c *IngestController) ensureQueue(ctx context.Context, name string) (entity
233233
}
234234
if err := queueStore.Create(ctx, queue); err != nil {
235235
if !errors.Is(err, storage.ErrAlreadyExists) {
236-
return entity.Queue{}, fmt.Errorf("IngestController failed to persist queue %s: %w", name, err)
236+
return entity.Queue{}, fmt.Errorf("failed to persist queue %s: %w", name, err)
237237
}
238238
// Raced with a concurrent creator; read the canonical row.
239239
return queueStore.Get(ctx, name)
@@ -254,7 +254,7 @@ func (c *IngestController) advanceQueueLatestRequestID(ctx context.Context, queu
254254
if queueRow.LatestRequestID != "" {
255255
cmp, err := entity.CompareRequestID(queue, id, queueRow.LatestRequestID)
256256
if err != nil {
257-
return fmt.Errorf("IngestController failed to compare request ids for queue %s: %w", queue, err)
257+
return fmt.Errorf("failed to compare request ids for queue %s: %w", queue, err)
258258
}
259259
if cmp <= 0 {
260260
return nil
@@ -268,7 +268,7 @@ func (c *IngestController) advanceQueueLatestRequestID(ctx context.Context, queu
268268
if errors.Is(err, storage.ErrVersionMismatch) {
269269
continue
270270
}
271-
return fmt.Errorf("IngestController failed to update queue %s latest_request_id: %w", queue, err)
271+
return fmt.Errorf("failed to update queue %s latest_request_id: %w", queue, err)
272272
}
273273
return nil
274274
}

stovepipe/controller/process/process.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
106106
case entity.RequestStateProcessing:
107107
if err := c.publishBuild(ctx, request.ID); err != nil {
108108
metrics.NamedCounter(c.metricsScope, _opName, "publish_errors", 1)
109-
return fmt.Errorf("ProcessController failed to publish request %s to build: %w", request.ID, err)
109+
return fmt.Errorf("failed to publish request %s to build: %w", request.ID, err)
110110
}
111111
return nil
112112
case entity.RequestStateSuperseded:
@@ -152,7 +152,7 @@ func (c *Controller) processAccepted(ctx context.Context, request entity.Request
152152
if err != nil {
153153
// TODO(queueconfig): decide retryability when a real config store lands — is a
154154
// missing queue "drop" (non-retryable) or "retry until configured"?
155-
return fmt.Errorf("ProcessController failed to load queue config for %s: %w", request.Queue, err)
155+
return fmt.Errorf("failed to load queue config for %s: %w", request.Queue, err)
156156
}
157157

158158
return c.admitLatestHead(ctx, request, queueRow, cfg)
@@ -164,7 +164,7 @@ func (c *Controller) processAccepted(ctx context.Context, request entity.Request
164164
func (c *Controller) coalesce(ctx context.Context, request entity.Request, latestRequestID string) (bool, error) {
165165
cmp, err := entity.CompareRequestID(request.Queue, request.ID, latestRequestID)
166166
if err != nil {
167-
return false, fmt.Errorf("ProcessController failed to compare request ids for queue %s: %w", request.Queue, err)
167+
return false, fmt.Errorf("failed to compare request ids for queue %s: %w", request.Queue, err)
168168
}
169169
if cmp >= 0 {
170170
return false, nil
@@ -203,7 +203,7 @@ func (c *Controller) admitLatestHead(ctx context.Context, request entity.Request
203203
metrics.NamedCounter(c.metricsScope, _opName, "source_control_errors", 1,
204204
metrics.NewTag("stage", "resolve"),
205205
)
206-
return fmt.Errorf("ProcessController failed to resolve source control for queue %s: %w", request.Queue, err)
206+
return fmt.Errorf("failed to resolve source control for queue %s: %w", request.Queue, err)
207207
}
208208
}
209209

@@ -242,7 +242,7 @@ func (c *Controller) admitLatestHead(ctx context.Context, request entity.Request
242242

243243
if err := c.publishBuild(ctx, request.ID); err != nil {
244244
metrics.NamedCounter(c.metricsScope, _opName, "publish_errors", 1)
245-
return fmt.Errorf("ProcessController failed to publish request %s to build: %w", request.ID, err)
245+
return fmt.Errorf("failed to publish request %s to build: %w", request.ID, err)
246246
}
247247

248248
metrics.NamedCounter(c.metricsScope, _opName, "admitted", 1,
@@ -281,7 +281,7 @@ func (c *Controller) deriveBuildStrategy(ctx context.Context, sc sourcecontrol.S
281281
metrics.NamedCounter(c.metricsScope, _opName, "source_control_errors", 1,
282282
metrics.NewTag("stage", "ancestry"),
283283
)
284-
return entity.BuildStrategyUnknown, "", fmt.Errorf("ProcessController failed to check ancestry for queue %s: %w", request.Queue, err)
284+
return entity.BuildStrategyUnknown, "", fmt.Errorf("failed to check ancestry for queue %s: %w", request.Queue, err)
285285
}
286286

287287
if isAncestor {
@@ -302,12 +302,12 @@ func (c *Controller) claimBuildSlot(ctx context.Context, queueRow *entity.Queue)
302302
if errors.Is(err, storage.ErrVersionMismatch) {
303303
got, getErr := queueStore.Get(ctx, queueRow.Name)
304304
if getErr != nil {
305-
return fmt.Errorf("ProcessController failed to reload queue %s after version mismatch: %w", queueRow.Name, getErr)
305+
return fmt.Errorf("failed to reload queue %s after version mismatch: %w", queueRow.Name, getErr)
306306
}
307307
*queueRow = got
308308
return storage.ErrVersionMismatch
309309
}
310-
return fmt.Errorf("ProcessController failed to claim build slot for queue %s: %w", queueRow.Name, err)
310+
return fmt.Errorf("failed to claim build slot for queue %s: %w", queueRow.Name, err)
311311
}
312312
updated.Version = newVersion
313313
*queueRow = updated
@@ -336,12 +336,12 @@ func (c *Controller) markProcessing(ctx context.Context, request *entity.Request
336336
if errors.Is(err, storage.ErrVersionMismatch) {
337337
got, getErr := reqStore.Get(ctx, request.ID)
338338
if getErr != nil {
339-
return false, fmt.Errorf("ProcessController failed to reload request %s after version mismatch: %w", request.ID, getErr)
339+
return false, fmt.Errorf("failed to reload request %s after version mismatch: %w", request.ID, getErr)
340340
}
341341
*request = got
342342
continue
343343
}
344-
return false, fmt.Errorf("ProcessController failed to mark request %s processing: %w", request.ID, err)
344+
return false, fmt.Errorf("failed to mark request %s processing: %w", request.ID, err)
345345
}
346346
updated.Version = newVersion
347347
*request = updated
@@ -402,12 +402,12 @@ func (c *Controller) supersedeRequest(ctx context.Context, request entity.Reques
402402
if errors.Is(err, storage.ErrVersionMismatch) {
403403
got, getErr := reqStore.Get(ctx, request.ID)
404404
if getErr != nil {
405-
return fmt.Errorf("ProcessController failed to reload request %s after version mismatch: %w", request.ID, getErr)
405+
return fmt.Errorf("failed to reload request %s after version mismatch: %w", request.ID, getErr)
406406
}
407407
request = got
408408
continue
409409
}
410-
return fmt.Errorf("ProcessController failed to supersede request %s: %w", request.ID, err)
410+
return fmt.Errorf("failed to supersede request %s: %w", request.ID, err)
411411
}
412412
return nil
413413
}
@@ -418,12 +418,12 @@ func (c *Controller) supersedeRequest(ctx context.Context, request entity.Reques
418418
func (c *Controller) rescheduleProcess(ctx context.Context, request entity.Request, inFlightCount int32, delayMs int64) error {
419419
if delayMs <= 0 {
420420
metrics.NamedCounter(c.metricsScope, _opName, "config_errors", 1)
421-
return fmt.Errorf("ProcessController requires a positive gate wait delay for queue %s, got %dms", request.Queue, delayMs)
421+
return fmt.Errorf("requires a positive gate wait delay for queue %s, got %dms", request.Queue, delayMs)
422422
}
423423

424424
payload, err := stovepipemq.Marshal(&stovepipemq.ProcessRequest{Id: request.ID})
425425
if err != nil {
426-
return fmt.Errorf("ProcessController failed to serialize process request %s: %w", request.ID, err)
426+
return fmt.Errorf("failed to serialize process request %s: %w", request.ID, err)
427427
}
428428

429429
// Suffix the message id with the publish time so the reschedule can't collide with
@@ -442,7 +442,7 @@ func (c *Controller) rescheduleProcess(ctx context.Context, request entity.Reque
442442

443443
if err := q.Publisher().PublishAfter(ctx, topicName, msg, delayMs); err != nil {
444444
metrics.NamedCounter(c.metricsScope, _opName, "publish_errors", 1)
445-
return fmt.Errorf("ProcessController failed to reschedule process request %s: %w", request.ID, err)
445+
return fmt.Errorf("failed to reschedule process request %s: %w", request.ID, err)
446446
}
447447
c.logger.Infow("rescheduled latest head awaiting build slot",
448448
"request_id", request.ID,
@@ -456,12 +456,12 @@ func (c *Controller) rescheduleProcess(ctx context.Context, request entity.Reque
456456

457457
// loadRequest returns the request for id.
458458
func (c *Controller) loadRequest(ctx context.Context, id string) (entity.Request, error) {
459-
return loader.ByID(ctx, id, c.store.GetRequestStore().Get, "ProcessController", "request")
459+
return loader.ByID(ctx, id, c.store.GetRequestStore().Get, "request")
460460
}
461461

462462
// loadQueue returns the queue row for name.
463463
func (c *Controller) loadQueue(ctx context.Context, name string) (entity.Queue, error) {
464-
return loader.ByID(ctx, name, c.store.GetQueueStore().Get, "ProcessController", "queue")
464+
return loader.ByID(ctx, name, c.store.GetQueueStore().Get, "queue")
465465
}
466466

467467
// publishBuild publishes the admitted request ID to the build stage. The build

stovepipe/core/loader/loader.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,8 @@ import (
2222
)
2323

2424
// ByID loads one entity by id via get, returning it unwrapped on success. On
25-
// failure it wraps the error as "<controllerName> failed to load <entityName>
26-
// <id>: <cause>" so every stovepipe controller reports load failures in the
27-
// same shape.
25+
// failure it wraps the error as "failed to load <entityName> <id>: <cause>"
26+
// so every stovepipe controller reports load failures in the same shape.
2827
//
2928
// get is typically a store's Get method value passed directly (e.g.
3029
// c.store.GetRequestStore().Get), which fixes T through inference so callers
@@ -36,11 +35,11 @@ import (
3635
// causally-prior write should already have produced is a storage
3736
// implementation defect, not a lag condition worth retrying through. It
3837
// surfaces as a plain error, non-retryable by platform/errs's default.
39-
func ByID[T any](ctx context.Context, id string, get func(context.Context, string) (T, error), controllerName, entityName string) (T, error) {
38+
func ByID[T any](ctx context.Context, id string, get func(context.Context, string) (T, error), entityName string) (T, error) {
4039
got, err := get(ctx, id)
4140
if err != nil {
4241
var zero T
43-
return zero, fmt.Errorf("%s failed to load %s %s: %w", controllerName, entityName, id, err)
42+
return zero, fmt.Errorf("failed to load %s %s: %w", entityName, id, err)
4443
}
4544
return got, nil
4645
}

0 commit comments

Comments
 (0)