firebase / firebase-admin-go

Firebase Admin Go SDK
Apache License 2.0
1.15k stars 249 forks source link

Allow package consumers to simulate errors in test #416

Open bpicolo opened 3 years ago

bpicolo commented 3 years ago

[REQUIRED] Step 2: Describe your environment

[REQUIRED] Step 3: Describe the problem

FirebaseError, along with it's relevant constructor, currently live inside package internal. errorutils gives a handy way to check if an error is a specific type of firebase error, but it's not possible to simulate these errors in unit tests in go, because consumers of the library don't have the ability to create error instances.

This means that usages of errorutils can't be unit tested directly - one would have to mock outbound http calls and hope to generate modern response bodies that Firebase creates, which is probably a good deal less desirable.

Do you have a recommended approach to testing simulated error conditions, today? Would it be possible to expand available error handling interfaces/APIs in a way that allows library consumers to simulate error conditions in test?

hiranya911 commented 3 years ago

Thanks for reporting this issue. I don't think there's an easy way to reproduce those error conditions in unit tests today. But we just added emulator support in #414. When this feature gets released in the near future, testing some of these error flows should become easier.

However, we'd probably want to go one step further, and make it trivial to create arbitrary errors for unit testing without the emulator as well. This will likely require adding internal.FirebaseError and internal.ErrorCode types to the public API surface. Although this sounds like more of a long-term proposal at this point.

@bpicolo can I ask you to add some more context to this issue? Perhaps some unit test code, and what it should look like had the SDK offered the capability to create arbitrary errors?

bpicolo commented 3 years ago

@hiranya911

We're wrapping most of the client usage inside an interface:

type FirebaseClient interface {
    GetUsersByEmail(ctx, email) (record, error)
}

By mocking that interface, we can customize the error returned, which is rad, but the goal in the calling code is to handle certain error conditions differently than others, i.e. EMAIL_EXISTS. The errorutils give us a way to check for that with IsAlreadyExists(), but we can't contrive a test for that code if we make use of that helper right now.

A statically-available instance of an error for each code would be sufficient as well - the goal would really be able to test these different error conditions: https://github.com/firebase/firebase-admin-go/blob/cef91acd46f2fc5d0b3408d8154a0005db5bdb0b/auth/user_mgt.go#L1069

hiranya911 commented 3 years ago

Thanks @bpicolo for the feedback. Making our SDKs more amenable for unit testing is one of our planned objectives this year. We will definitely consider this requirement as part of that.

A statically-available instance of an error for each code would be sufficient as well

That's an interesting idea. I guess all you need is the ability to reference or create a value that will pass our IsXXXXX() error checks in the errorutils package. Perhaps we ought to provide a series of factory methods in errorutils to create new error instances. I see a similar pattern in some of the other Go packages that I've referenced.

tyrone-anz commented 2 years ago

@hiranya911 I deleted my old reply here and added this one as it seems my understanding was wrong then.

Do you think it would be possible to get the FirebaseError exported? I wanted to record the different messagingErrorCode I am receiving from FCM. Would be fine as well if there's an errorutils function to extract that code in particular from an error.

sashayakovtseva commented 1 year ago

I am having the same issue here. We have complex error handling code and want to test it, but FirebaseError is still in internal package.

@lahirumaramba @hiranya911 any news? It's been almost two years since the original proposal and nothing changed.

abrenn commented 1 year ago

@lahirumaramba @hiranya911 since it's another four months since the last ping I will ping you again. Please export the FirebaseError type so people are able to unit test their code.

mauriciovillam commented 1 year ago

Hello, any update on this? I am not able to test the sad paths of my authentication process.

poporul commented 5 months ago

Hi, it has been year since last comment. I will try my luck and ask do you have any updates about exporting internal.FirebaseError @lahirumaramba?