From c6af159b42acd12fb2bd7c6bc9b1daee6497af3a Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Mon, 19 Aug 2024 14:23:50 -0300 Subject: [PATCH] Fix strange relationship between local contexts in ... ... handlingLoop[Command].run(). This method derives two context ojects mirroring the structure found in the Server type: `gCtx` for graceful stop and `ctx` for "brute forcing". Those local context objects must be derived from the stream context, so they can both be used for logging. They were arrenged in such a way that the local `gCtx` is child of `ctx`. If the server own `ctx` is cancelled to force a stop, depending on the moment that happens it could be interpreted by this method as a graceful stop (because it only checks `gCtx` and that can be cancelled due its parent being cancelled). That wouldn't occur if the `gCtx` was sibling of `ctx` (thus direct child of `h.stream.Context()`), which should be enough for the logging use case. The whole point here is that `server.Stop()` is assumed in tests to cause `server.Serve()` to return a non-nil error. But that can only happen if the handling loop objects fail or perceive a brute-force cancellation. If `handlingLooop[Command].run()` perceives `server.Stop()` as a graceful cancellation, then they won't return errors. Whether `server.Stop()` implies in a graceful cancellation for the handling loops was just a matter of in which moment of the for{} the notification arrived. With the decoupling herein presented, that should never be possible. --- wsl-pro-service/internal/streams/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wsl-pro-service/internal/streams/server.go b/wsl-pro-service/internal/streams/server.go index 11f477c61..ae9181491 100644 --- a/wsl-pro-service/internal/streams/server.go +++ b/wsl-pro-service/internal/streams/server.go @@ -182,7 +182,7 @@ func (h *handlingLoop[Command]) run(s *Server, client *multiClient) error { defer cancel() // Use this context to log onto the stream, but cancel with server.GracefulStop - gCtx, cancel := cancelWith(ctx, s.gracefulCtx) + gCtx, cancel := cancelWith(h.stream.Context(), s.gracefulCtx) defer cancel() for {