4
\$\begingroup\$

I am currently "gracefully" handling HTTP server net/http shutdown which works perfectly fine (I am not asking you to review server shutdown). As seen in the code below the context is created first time directly in server package when I run the application. What I've done afterwards is (this is the one I would like you to review please), I've created a context.Background() before calling the server and pass it to server. Is this acceptable/feasible practise in Golang?

Why I want to create a context beforehand while everything is already working fine?

  • It is because I need a few key-value pair in context so that I can use them throughout the application.

CURRENT IMPLEMENTATION WITH CONTEXT IN SERVER

cmd/auth/main.go

package main

import (
    "github.com/BentCoder/auth/internal/http"
)

func main() {
    srv := http.NewServer(":8011", 10)
    srv.Start()
}

internal/http/server.go

package http

import (
    "context"
    "log"
    "net/http"
    "os"
    "os/signal"
    "syscall"
    "time"
)

type Server struct {
    http    *http.Server
    timeout int64
}

func NewServer(adr string, tim int64) Server {
    return Server{
        http: &http.Server{
            Addr: adr,
        },
        timeout: tim,
    }
}

func (srv *Server) Start() {
    log.Infof("starting HTTP server")

    idle := make(chan struct{})

    go shutdown(srv.http, idle, srv.timeout)

    if err := srv.http.ListenAndServe(); err != http.ErrServerClosed {
        log.Fatalf("failed to start/close HTTP server [%v]", err)
    }

    <-idle

    log.Info("shutdown HTTP server")
}

func shutdown(srv *http.Server, idle chan<- struct{}, tim int64) {
    sig := make(chan os.Signal, 1)

    signal.Notify(sig, os.Interrupt, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP)

    <-sig

    log.Infof("shutting down HTTP server in '%d' sec", tim)

    ctx, cancel := context.WithTimeout(context.Background(), time.Duration(tim)*time.Second)
    defer cancel()

    if err := srv.Shutdown(ctx); err != nil {
        log.Fatalf("failed to shutdown HTTP server [%v]", err)
    }

    close(idle)
}

output

INFO[0000] starting HTTP server                          source="server.go:42"
^C
INFO[0004] shutting down HTTP server in '10' sec         source="server.go:82"
INFO[0004] shutdown HTTP server                          source="server.go:54"

THIS IS WHAT I DECIDED TO DO

package main

import (
    "context"
    "github.com/BentCoder/auth/internal/http"
)

func main() {
    ctx := context.WithValue(context.Background(), "SomeKey", "SomeVal")

    srv := http.NewServer(":8011", 10)
    srv.Start(ctx)
}

And update server package (the relevant bit) as seen below.

func shutdown(ctx context.Context, .....
    ...
    ctx, cancel := context.WithTimeout(ctx, time.Duration(tim)*time.Second)
    ...
}
\$\endgroup\$

bumped to the homepage by Community yesterday

This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.

0
\$\begingroup\$

You've actually improved the design, because now unit tests can inject their own context and thus the shutdown() becomes testable. Pure gold.

But your stated reasons make me wary. I'd pass a parameter (e.g. struct) to Start() or NewServer() There aren't many good uses of WithValue that make your code more readable and neater.

Also docs for WithValue warn that "The provided key must be comparable and should not be of type string or any other built-in type to avoid collisions between packages using context." So that's a big problem with your example.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.