fdk-go icon indicating copy to clipboard operation
fdk-go copied to clipboard

502 on error

Open harryjsmith opened this issue 5 years ago • 2 comments

Currently the Go FDK doesn't handle errors in a user's function. As part of the FDK contract, the FDK should emit a 502 on a detectable error from the function. Not sure what the most idiomatic way of doing this from a user's perspective is. It could possibly involve the user setting a response status (500?) which the FDK then interprets as an error and gives a 502?

harryjsmith avatar Nov 27 '18 10:11 harryjsmith

Well, this is what you should do by yourself, because it's up to a developer to handle his errors. Here's very basic prototype:

// here you can do whatever you want
func myHandler(ctx context.Context, in io.Reader, out io.Writer) error {
        return errors.New("blah-blah-error")
}

// here you can handle errors as you wish
func withError(ctx context.Context, in io.Reader, out io.Writer) {
        err := myHandler(ctx, in, out)
        if err != nil {
                fdk.WriteStatus(out, http.StatusInternalServerError)
                io.WriteString(out, err.Error())
                return
        }
        fdk.WriteStatus(out, http.StatusOK)
}

func main() {
	fdk.Handle(fdk.HandlerFunc(withError))
}

BTW, with this ^ particular approach you're making your code more suitable for testing without asking FDK for test fixtures.

denismakogon avatar Nov 27 '18 11:11 denismakogon

yea, for http triggers it makes sense to force users to handle them with their own error codes and errors imo, otherwise we end up imposing some default format on people there (a json message, eg) - managing the divide between invoke and http trigger is kinda weird. for users that do that with http triggers, if they invoke the function with the invoke contract they'll actually break the invoke contract unless they end up using 502 or 504, which kinda stinks, that onus shouldn't be on users - we could likely handle this at the fdk level if we get some > 400, but it's a bit magical. if we add an error to the handler we'd need a contract to define that if a user wrote to the output and returns an error, we'd have to establish precedence there for which gets actually written back out (until we add streaming) - it was something we were trying to avoid anyway. denis' code above maybe helps highlight the issue, if the handler writes to the output and then returns an error it will write both the output and the error (possibly a double error) - say, for somebody that wants to return 503 when their db is down, managing these 2 ways of handling errors is a bit cumbersome (even if we work some magic underneath the covers, it has to be reasoned about by users). open to concrete proposals on this though.

rdallman avatar Nov 27 '18 18:11 rdallman