watermill icon indicating copy to clipboard operation
watermill copied to clipboard

Checking for argument == nil wont work for interfaces

Open dubyte opened this issue 3 years ago • 0 comments

When passing a nil DB the code that checks the db == nill is false because the beginner is an interface so the nil DB satisfies the interface but is nil. See this link for reference. https://glucn.medium.com/golang-an-interface-holding-a-nil-value-is-not-nil-bb151f472cc7


func NewSubscriber(db beginner, config SubscriberConfig, logger watermill.LoggerAdapter) (*Subscriber, error) {
	if db == nil { // <- this is always false even if we sent a nil db
		return nil, errors.New("db is nil")
	}

So in other words there is an assumption that we are caching the case of receiving a nil value and returning an error but db == nill return false in all cases.

The next page explores some ways to check that an interface is nil: https://mangatmodi.medium.com/go-check-nil-interface-the-right-way-d142776edef1

dubyte avatar Jun 07 '22 13:06 dubyte

In those examples the function definition calls for an empty interface. That is not the case in the NewSubscriber function.

thpk avatar Aug 25 '22 16:08 thpk

could you please give an example where NewSubscriber returns error: errors.New("db is nil")

The issue here is that there is not possible to get that error, with any value of db.

dubyte avatar Aug 25 '22 18:08 dubyte

The trivial example would throw that error:

_, err := sql.NewSubscriber(nil, sql.SubscriberConfig{}, nil)
fmt.Println(err)

any initialized value of db wouldn't throw this error of course. This error check prevents panics in setup.

Edit for clarification:

say the method was func someMethod(db interface{}) error then db doesn't have a very explicit type. But in this case the method is more like func otherMethod(db beginner) error where db does have an explicit type: the interface beginner. Hope this helps :-)

thpk avatar Sep 08 '22 22:09 thpk

First at all thanks for your time. Is really appreciated

The thing is nil can not be passed because it won't compile, and in the moment you are sending a nil value of type begginer. The condition if db == nil does not evaluate as true.

so imagine unit test scenario is not possible to that NewSuscriber returns: errors.New("db is nil")

please try to get db is nil error in a snippet.

Thanks

dubyte avatar Sep 12 '22 18:09 dubyte

Have you tried the snippet above? It does work perfectly here. I suggest you clone the repository and add the the file subscriber_test.go:

package sql

import (
	"fmt"
	"testing"
)

func TestSubscriberNil(t *testing.T) {
	_, err := NewSubscriber(nil, SubscriberConfig{}, nil)
	fmt.Println(err)
}

don't forget that as a user of the package it is impossible to use beginner{} as argument, as beginner is unexported.

thpk avatar Sep 12 '22 22:09 thpk

Thank you, it works as you mentioned.

I can see now that the issue is on my code and not in the lib.

dubyte avatar Sep 13 '22 15:09 dubyte