0
\$\begingroup\$

I've been playing around with Go and finally found a use case for a little web service I could build.

It takes JSON data via an HTTP POST request and sends me an email via Mailgun's API (it's used for a contact form on my website).

I'm new to Go and would love some feedback on what I could do better, anything I missed and where I could do better following the "Go way" of doing things. :)

main.go

package main

import (
    "fmt"
    "log"
    "net/http"
    "os"

    "github.com/gorilla/handlers"
    "github.com/gorilla/mux"
)

var mailgunDomain string = os.Getenv("MAILGUN_DOMAIN")
var mailgunAPIKey string = os.Getenv("MAILGUN_API_KEY")
var allowedOrigin string = os.Getenv("ALLOWED_ORIGIN")
var port string = os.Getenv("PORT")

func main() {
    if port == "" {
        panic("Missing environment variable PORT")
    }
    if mailgunDomain == "" {
        panic("Missing environment variable MAILGUN_DOMAIN")
    }
    if mailgunAPIKey == "" {
        panic("Missing environment variable MAILGUN_API_KEY")
    }
    if allowedOrigin == "" {
        panic("Missing environment variable ALLOWED_ORIGIN")
    }

    r := mux.NewRouter()

    r.Use(handlers.CORS(
        handlers.AllowedHeaders([]string{"Content-Type"}),
        handlers.AllowedOrigins([]string{allowedOrigin}),
    ))

    r.HandleFunc("/", homeHandler).Methods("GET")
    r.HandleFunc("/contact", HandleContact(mailgunDomain, mailgunAPIKey)).Methods("POST", "OPTIONS")

    log.Fatal(http.ListenAndServe(":"+port, r))
}

func homeHandler(w http.ResponseWriter, r *http.Request) {
    w.WriteHeader(http.StatusOK)
    fmt.Fprintln(w, "API for maxschmitt.me")
}

contact.go

package main

import (
    "context"
    "encoding/json"
    "fmt"
    "net/http"
    "os"
    "time"

    "github.com/mailgun/mailgun-go/v4"
)

var mailgunSender string = "[email protected]"
var mailgunReceiver string = "[email protected]"

// HandleContact sends emails to me via the contact form
func HandleContact(mailgunDomain string, mailgunAPIKey string) http.HandlerFunc {
    type body struct {
        Name    string `json:"name"`
        Email   string `json:"email"`
        Message string `json:"message"`
    }

    mg := mailgun.NewMailgun(mailgunDomain, mailgunAPIKey)
    mg.SetAPIBase(mailgun.APIBaseEU)

    return func(w http.ResponseWriter, r *http.Request) {
        var b body

        err := json.NewDecoder(r.Body).Decode(&b)
        if err != nil {
            http.Error(w, err.Error(), http.StatusBadRequest)
            return
        }

        message := "From: " + b.Name + "<" + b.Email + ">\n\n"
        message += b.Message

        fmt.Fprintln(os.Stdout, "## New message!")
        fmt.Fprintln(os.Stdout, message+"\n")

        mgMsg := mg.NewMessage(mailgunSender, "New contact form submission", message, mailgunReceiver)
        mgMsg.SetReplyTo(b.Email)

        // Create a 10s timeout for sending the message
        ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
        defer cancel()

        resp, id, err := mg.Send(ctx, mgMsg)
        if err != nil {
            fmt.Println("Error sending contact form submission:")
            fmt.Println(err)
            http.Error(w, "Something went wrong", http.StatusInternalServerError)
            return
        }

        fmt.Fprintln(os.Stdout, "Mailgun response:", resp)
        fmt.Fprintln(os.Stdout, "Mailgun ID:", id)

        json.NewEncoder(w).Encode(b)
    }
}

I appreciate any feedback! Thanks!

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

A couple thoughts on contact.go.

Global Variables

In general, it's advised to not use global variables like

var mailgunSender string = "[email protected]"
var mailgunReceiver string = "[email protected]"

Instead, there are a few options. You already have a struct definition inside the HandleContact closure, so you could just move them there. You could also pass them as arguments to HandleContact().

The other alternative could be to define a Mailgun struct with those as fields (and could probably include the mailgunDomain and mailgunAPIKey as well). Then define HandleContact as a method on the Mailgun struct, and use the handler by first creating a new Mailgun with the appropriate values like so:

main.go

mg := Mailgun{
    Sender:   "[email protected]",
    Receiver: "[email protected]",
    Domain:   mailgunDomain,
    ApiKey:   mailgunAPIKey,
}
r.HandleFunc("/contact", mg.HandleContact()).Methods("POST", "OPTIONS")

contact.go

type Mailgun struct {
    Sender:   string
    Receiver: string
    Domain:   string
    ApiKey:   string
}
func (m *Mailgun) HandleContact() http.HandlerFunc {
    type body struct {
        Name    string `json:"name"`
        Email   string `json:"email"`
        Message string `json:"message"`
    }

    mg := mailgun.NewMailgun(m.Domain, m.APIKey)
    //etc...

I think the biggest advantage to setting it up this way is that it becomes very testable. You can inject whatever settings are appropriate for testing.

Printing

fmt.Printf() outputs to Stdout by default, so the lines

fmt.Fprintln(os.Stdout, "## New message!")
fmt.Fprintln(os.Stdout, message+"\n")
// ...
fmt.Fprintln(os.Stdout, "Mailgun response:", resp)
fmt.Fprintln(os.Stdout, "Mailgun ID:", id)

are functionally the same as

// Note 2 newlines to keep output identical to your implementation
fmt.Printf("## New message!\n %s\n\n", message) 
// ...
fmt.Printf("Mailgun response: %s\n", resp)
fmt.Printf("Mailgun ID: %s\n", id)

Errors

Don't forget to handle the error from json.NewEncoder(w).Encode(b) at the end of contact.go. As the code is written I don't see any reason why it should ever return an error, you're just re-encoding the body you just decoded and echoing it back to the client. But if you ever change your code and the body is modified in some way it could fail silently and lead to debugging headaches.

Also wondering what the purpose of re-encoding is, you could just store and re-use the original value of r.Body. Did you mean to encode and return the response from Mailgun?

\$\endgroup\$
1
  • \$\begingroup\$ Nice first answer! \$\endgroup\$
    – pacmaninbw
    Mar 20, 2021 at 11:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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