5
\$\begingroup\$

I want to collect errors and its causes by defining custom errors (HTTP status 40x & 50x). For example, if user's request was bad, I want more specific reasons for the error.

A customized error has its own code (not equal to HTTP status code, but related since the code format is $HTTP_STATUS (3-digit) + own code (2-digit). The reason it has its own error code is that I store this error data (as JSON) to Elasticsearch and query for each error's occurrences in Kibana.

I'd like to get reviews on the following codes. Best practice or any small tips for custom errors are welcome.

error.go :

package main

import (
    "encoding/json"
    "fmt"
    "net/http"
)

const (
    codeEmptyNameErr        = 40001
    codeInvalidAgeErr       = 40002
    codeMethodNotAllowedErr = 40501
    codeJSONMarshalErr      = 50001
)

// MyError contains custom code, error message, and HTTP status code.
type MyError struct {
    HTTPStatus int    `json:"-"`
    Code       int    `json:"code"`
    Message    string `json:"message"`
}

func (e *MyError) Error() string {
    return fmt.Sprintf("HTTPStatus: %v, Code: %v, Message: %q",
        e.HTTPStatus, e.Code, e.Message)
}

// WriteToResponse writes response for the error.
func (e *MyError) WriteToResponse(w http.ResponseWriter) {
    w.WriteHeader(e.HTTPStatus)
    fmt.Fprintf(w, e.ToJSON())
    // TODO: store e.ToJSON() to ElasticSearch for future analysis
}

// ToJSON returns JSON string for a MyError.
func (e *MyError) ToJSON() string {
    j, err := json.Marshal(e)
    if err != nil {
        return `{"code":50099,"message":"ScrapError.JSONStr: json.Marshal() failed"}`
    }
    return string(j)
}

// EmptyNameErr .
func EmptyNameErr(name string) *MyError {
    return &MyError{
        HTTPStatus: http.StatusBadRequest,
        Code:       codeEmptyNameErr,
        Message:    "name shoud not be empty",
    }
}

// InvalidAgeErr .
func InvalidAgeErr(age string) *MyError {
    return &MyError{
        HTTPStatus: http.StatusBadRequest,
        Code:       codeInvalidAgeErr,
        Message:    fmt.Sprintf("invalid age: %q", age),
    }
}

// MethodNotAllowedErr .
func MethodNotAllowedErr(method string) *MyError {
    return &MyError{
        HTTPStatus: http.StatusMethodNotAllowed,
        Code:       codeMethodNotAllowedErr,
        Message:    fmt.Sprintf("method %q is not allowed", method),
    }
}

// JSONMarshalErr .
func JSONMarshalErr(err error) *MyError {
    return &MyError{
        HTTPStatus: http.StatusInternalServerError,
        Code:       codeJSONMarshalErr,
        Message:    err.Error(),
    }
}

main.go :

package main

import (
    "encoding/json"
    "fmt"
    "log"
    "net/http"
    "strconv"
)

func main() {
    // request example:
    // curl -X POST -d 'name=John%20Doe&age=10' localhost:8083/persons
    http.HandleFunc("/persons", newPersonHandler)
    log.Fatal(http.ListenAndServe(":8083", nil))
}

func newPersonHandler(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json; charset=utf-8")
    switch r.Method {
    case "POST":
        name, ageStr := r.FormValue("name"), r.FormValue("age")
        if name == "" {
            EmptyNameErr(name).WriteToResponse(w)
            return
        }
        age, err := strconv.ParseUint(ageStr, 10, 64)
        if err != nil {
            InvalidAgeErr(ageStr).WriteToResponse(w)
            return
        }

        resBody, err := json.Marshal(person{name, age})
        if err != nil {
            JSONMarshalErr(err).WriteToResponse(w)
            return
        }

        fmt.Fprintf(w, string(resBody))
    default:
        MethodNotAllowedErr(r.Method).WriteToResponse(w)
    }
}

type person struct {
    Name string `json:"name"`
    Age  uint64 `json:"age"`
}
\$\endgroup\$
2
\$\begingroup\$

If you have a status code that incorporates the HTTP one, how about you have a splitter that will you give you the HTTP one from the custom one?

Then again, if you're returning JSON, why not an enum (well, a string, sure) instead of an opaque error code? I suppose it might be more efficient to query and quicker if you know what you're looking for, but then again Elasticsearch is supposed to be quick to query for such things, isn't it.

\$\endgroup\$
2
\$\begingroup\$

Your code is fine, but don't use fmt.Fprintf if you don't need formatting.

It will scan your string for placeholders (like %s):

fmt.Fprintf(os.Stdout, "100%sure")
// 100%!s(MISSING)ure

Use Write directly:

os.Stdout.Write([]byte("100%sure"))
// 100%sure

And don't forget to handle returned error.

A bit more about io.Writer

Quote from documentation:

Writer is the interface that wraps the basic Write method.

Write writes len(p) bytes from p to the underlying data stream. It returns the number of bytes written from p (0 <= n <= len(p)) and any error encountered that caused the write to stop early. Write must return a non-nil error if it returns n < len(p). Write must not modify the slice data, even temporarily.

I guess that implementations of Write method are encouraged to write all data in a single call. If we digg the sources a bit we'll find, that under the hood most Write methods are implemented with loops.

On the other hand io.Reader may return with partial read and nil error, so methods like io.ReadFull and ioutil.ReadAll are quite handy.

\$\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.