drsybren/Flamenco
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Manager: avoid logging error on HTTP disconnect on some API calls

Browse Source 
Improve the error handling on some worker management API calls, to deal
with closed HTTP connections better.

A new function, `api_impl.handleConnectionClosed()` can now be called when
`errors.Is(err, context.Canceled)`. This will only log at debug level, and
send a `419 I'm a Teapot` response to the client. This response will very
likely never be seen, as the connection was closed. However, in case this
function is called by mistake, this response is unlikely to be accepted
by the HTTP client.
This commit is contained in:
Sybren A. Stüvel 2024-06-26 10:25:10 +02:00
parent 125e9aba23
commit 6c2d3d7fc0
2 changed files with 36 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
internal/manager/api_impl/api_impl.go
View File

@ -12,6 +12,7 @@ import (
"time"
"github.com/labstack/echo/v4"
"github.com/rs/zerolog"
"projects.blender.org/studio/flamenco/pkg/api"
)
@ -134,3 +135,12 @@ func sendAPIErrorDBBusy(e echo.Context, message string, args ...interface{}) err
e.Response().Header().Set("Retry-After", strconv.FormatInt(seconds, 10))
return e.JSON(code, apiErr)
}
// handleConnectionClosed logs a message and sends a "418 I'm a teapot" response
// to the HTTP client. The response is likely to be seen, as the connection was
// closed. But just in case this function was called by mistake, it's a response
// code that is unlikely to be accepted by the client.
func handleConnectionClosed(e echo.Context, logger zerolog.Logger, logMessage string) error {
logger.Debug().Msg(logMessage)
return e.NoContent(http.StatusTeapot)
}

41
internal/manager/api_impl/worker_mgt.go
View File

@ -3,6 +3,7 @@ package api_impl
// SPDX-License-Identifier: GPL-3.0-or-later
import (
"context"
"errors"
"net/http"
@ -16,7 +17,10 @@ import (
func (f *Flamenco) FetchWorkers(e echo.Context) error {
logger := requestLogger(e)
dbWorkers, err := f.persist.FetchWorkers(e.Request().Context())
if err != nil {
switch {
case errors.Is(err, context.Canceled):
return handleConnectionClosed(e, logger, "fetching all workers")
case err != nil:
logger.Error().Err(err).Msg("fetching all workers")
return sendAPIError(e, http.StatusInternalServerError, "error fetching workers: %v", err)
}
@ -42,18 +46,23 @@ func (f *Flamenco) FetchWorker(e echo.Context, workerUUID string) error {
ctx := e.Request().Context()
dbWorker, err := f.persist.FetchWorker(ctx, workerUUID)
if errors.Is(err, persistence.ErrWorkerNotFound) {
switch {
case errors.Is(err, persistence.ErrWorkerNotFound):
logger.Debug().Msg("non-existent worker requested")
return sendAPIError(e, http.StatusNotFound, "worker %q not found", workerUUID)
}
if err != nil {
case errors.Is(err, context.Canceled):
return handleConnectionClosed(e, logger, "fetching task assigned to worker")
case err != nil:
logger.Error().Err(err).Msg("fetching worker")
return sendAPIError(e, http.StatusInternalServerError, "error fetching worker: %v", err)
}
dbTask, err := f.persist.FetchWorkerTask(ctx, dbWorker)
if err != nil {
logger.Error().Err(err).Msg("fetching task assigned to worker")
switch {
case errors.Is(err, context.Canceled):
return handleConnectionClosed(e, logger, "fetching task assigned to worker")
case err != nil:
logger.Error().AnErr("cause", err).Msg("fetching task assigned to worker")
return sendAPIError(e, http.StatusInternalServerError, "error fetching task assigned to worker: %v", err)
}
@ -86,11 +95,11 @@ func (f *Flamenco) DeleteWorker(e echo.Context, workerUUID string) error {
// Fetch the worker in order to re-queue its tasks.
worker, err := f.persist.FetchWorker(ctx, workerUUID)
if errors.Is(err, persistence.ErrWorkerNotFound) {
switch {
case errors.Is(err, persistence.ErrWorkerNotFound):
logger.Debug().Msg("deletion of non-existent worker requested")
return sendAPIError(e, http.StatusNotFound, "worker %q not found", workerUUID)
}
if err != nil {
case err != nil:
logger.Error().Err(err).Msg("fetching worker for deletion")
return sendAPIError(e, http.StatusInternalServerError,
"error fetching worker for deletion: %v", err)
@ -105,11 +114,11 @@ func (f *Flamenco) DeleteWorker(e echo.Context, workerUUID string) error {
// Actually delete the worker.
err = f.persist.DeleteWorker(ctx, workerUUID)
if errors.Is(err, persistence.ErrWorkerNotFound) {
switch {
case errors.Is(err, persistence.ErrWorkerNotFound):
logger.Debug().Msg("deletion of non-existent worker requested")
return sendAPIError(e, http.StatusNotFound, "worker %q not found", workerUUID)
}
if err != nil {
case err != nil:
logger.Error().Err(err).Msg("deleting worker")
return sendAPIError(e, http.StatusInternalServerError, "error deleting worker: %v", err)
}
@ -145,11 +154,13 @@ func (f *Flamenco) RequestWorkerStatusChange(e echo.Context, workerUUID string)
// Fetch the worker.
dbWorker, err := f.persist.FetchWorker(e.Request().Context(), workerUUID)
if errors.Is(err, persistence.ErrWorkerNotFound) {
switch {
case errors.Is(err, context.Canceled):
return handleConnectionClosed(e, logger, "fetching worker")
case errors.Is(err, persistence.ErrWorkerNotFound):
logger.Debug().Msg("non-existent worker requested")
return sendAPIError(e, http.StatusNotFound, "worker %q not found", workerUUID)
}
if err != nil {
case err != nil:
logger.Error().Err(err).Msg("fetching worker")
return sendAPIError(e, http.StatusInternalServerError, "error fetching worker: %v", err)
}