Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces cross-process context propagation for distributed task execution by adding a ContextCodec interface and implementing it for call contexts (user identity, service parameters, and tenant data). It also enhances the work queue infrastructure by implementing concurrency limiting via a new semaphore utility across in-memory, pull, and push queue variants, and adds execution callbacks to the pull queue. The review feedback highlights several critical issues: potential nil pointer dereferences in the work queues if used before handler registration, unreachable error handling logic in the pull queue, and data integrity concerns regarding shallow copies of service parameters and fragile type assertions during context decoding.
| } | ||
|
|
||
| if handleErr != nil { | ||
| if returnErr := msg.Return(ctx, handleErr); returnErr != nil { |
There was a problem hiding this comment.
If handleErr is ErrMalformedPayload, calling msg.Return contradicts the definition of ErrMalformedPayload as a non-retryable error. Returning it to the queue will cause it to be redelivered and fail again. Such errors should result in the message being completed (and optionally moved to a Dead Letter Queue).
workqueue.Messagestructworkqueue.NewInMemorylimiter.ConcurrencyConfighandling in workqueuesCallContextcodec so that service params and auth information can be restored for executionBeforeExecutionCallbackandAfterExecutionCallbackforworkqueue.PullQueueConfigas extension points for implementing custom context injection and execution result handling (complete or return a message)workqueue.WriterFuncfor creating writer decorators.AttachToContextinstead ofslog.SetDefaultwhich was problematic for parallel tests