rcrowley / go-tigertonic

A Go framework for building JSON web services inspired by Dropwizard
Other
997 stars 79 forks source link

Error Handler #89

Closed keithballdotnet closed 9 years ago

keithballdotnet commented 9 years ago

If I were to extract the current errors handling to an interface and allow it to be replaced, would that be something that might be pulled? Or would I be wasting my time?

mihasya commented 9 years ago

Could you flesh that out a little bit more? What's the interface you have in mind? I've noodled (fruitlessly) quite a bit on how to make returning status from a tigertonic handler, particularly one that fits into tigertonic.Marshaled, more DRY (which I presume is the goal here, let me know if that's not right)

keithballdotnet commented 9 years ago

Specifically, I would like to overwrite the function writeJSONError in error.go.

Rather than have the error marshalled into the current fixed form, I would like to replace the marshalling into a custom format.

willfaught commented 9 years ago

Are you planning to use this to customize the behavior of the If or Marshaled middleware? If so, could you instead write your own middleware that uses your specific format?

The README does say:

A Go framework for building JSON web services inspired by Dropwizard. If HTML is your game, this will hurt a little.

Maybe this is the "hurt a little" part? ;)

keithballdotnet commented 9 years ago

I have a custom error with error and sub error codes:

// Error wraps the code and message which defines an error. It also
// can be used to wrap an original error object.
type Error struct {
    // Satisfy the generic error interface.
    error

    // Classification of error
    Code ErrorCode

    // Sub code if any
    Subcode ErrorCode

    // Detailed information about error
    Message string
}

I would like to modify

func writeJSONError(w http.ResponseWriter, err error) {
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(errorStatusCode(err))
    if jsonErr := json.NewEncoder(w).Encode(map[string]string{
        "description": err.Error(),
        "error":       errorName(err, "error"),
    }); nil != jsonErr {
        log.Printf("Error marshalling error response into JSON output: %s", jsonErr)
    }
}

To use my custom error struct like so...

func writeJSONError(w http.ResponseWriter, err error) {
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(errorStatusCode(err))
    if jsonErr := json.NewEncoder(w).Encode(err); nil != jsonErr {
        log.Printf("Error marshalling error response into JSON output: %s", jsonErr)
    }
}

The best way to do this, I believe would be to have the current error handler as a interface which can be replaced by a custom implementation.

willfaught commented 9 years ago

Could you modify Marshaled to call your custom function, e.g. in these places:

Tiger Tonic works with any http.Handler, so you can pretty much do whatever you want.

mihasya commented 9 years ago

@Inflatablewoman so you're saying instead of relying on the global writeJsonError function, make it a method on Marshaler that can be overwritten? I'd merge that as long as it was done in a backwards compatible way, e.g. the default is to preserve the current behavior. Thanks for elaborating!

keithballdotnet commented 9 years ago

Aye, right. I'll have a look at it when I get chance and submit a pull request. Thanks for letting me know it (might be) worth the effort. :)

keithballdotnet commented 9 years ago

Pull request is now there.

https://github.com/rcrowley/go-tigertonic/pull/90

keithballdotnet commented 9 years ago

Thanks for the merge.

mihasya commented 9 years ago

:metal: no worries