revdotcom / revai-go

Rev.ai golang client
https://www.rev.ai/docs
MIT License
21 stars 12 forks source link

error codes #10

Closed jdupl123 closed 3 years ago

jdupl123 commented 3 years ago

Error handling for streams

threeaccents commented 3 years ago

@jdupl123 Hey man I'm gonna be offline for the next few weeks my first child was born so I've been quite busy haha.

A few thoughts on this before I go offline though. Error handling like this 100% needed.

Some changes I would make. Prepend Err to all the errors so

        ErrCloseUnauthorized        = 4001
    ErrCloseBadRequest          = 4002
    ErrCloseInsufficientCredits = 4003
    ErrCloseServerShuttingDown  = 4010
    ErrCloseNoInstanceAvailable = 4013
    ErrCloseTooManyRequests     = 4029

Let's also make a custom error type(ErrRetriable). So the user can just use go built in errors.Is to check if to retry or not.

if errors.Is(err, ErrRetriable) {
 // do logic
}

I'm not 100% sure on adding a second channel to handle errors. Just thinking of the flow of the program of the users perspective it would feel a bit off. If we could come up with a solution where it's not needed that could be nice. I wonder of other go packages that have similar streaming apis how they handle it.

Let me know what you think.

jdupl123 commented 3 years ago

@threeaccents Congratulations on your baby!

Happy to prepend Err.

ErrRetriable sounds good to me.

Yeah second channel feels a bit clunky to me too.

The google api provides a Recv method as part of the interface.

https://godoc.org/google.golang.org/genproto/googleapis/cloud/speech/v1#Speech_StreamingRecognizeClient

That way the response Msg and err can be returned as part of the same function call.

threeaccents commented 3 years ago

@jdupl123 I like that a lot with the Recv method. Let me know if you would like to implement that change or I can probably get on it in the next few days as well.

jdupl123 commented 3 years ago

I can put up a proposed Recv method soon.

jdupl123 commented 3 years ago

I've tested it locally now using the stream.go example and its working quite nicely.

threeaccents commented 3 years ago

@jdupl123 You the man this looks awesome! We can just add in the method docs about the unbuffered channel. Merging this in!