wealdtech / go-ens

Apache License 2.0
90 stars 33 forks source link

ReverseResolver obfuscates transient rpc errors when resolved name is empty string #21

Closed oldmanfleming closed 2 years ago

oldmanfleming commented 2 years ago

When calling the Name function on the reverse resolver contract, if an empty string is encountered, even if the error was a transient error, the original error is dropped in favor of errors.New("no resolution"). This makes it impossible to get access to the original rpc.HTTPError and decide whether there is really no reverse record for the given address or we simply encountered a transient error and should retry. For instance, It is common for providers like Alchemy to return a 429 status code indicating a rate-limit has been applied. In these scenarios we can check the rpc.HTTPError that is returned for a certain status code and decide for our selves whether we can retry.

Relevant code:

func ReverseResolve(backend bind.ContractBackend, address common.Address) (string, error) {
    resolver, err := NewReverseResolverFor(backend, address)
    if err != nil {
        return "", err
    }

    // Resolve the name
    name, err := resolver.Name(address)
    if name == "" {
        err = errors.New("no resolution")
    }

    return name, err
}

It would be more favorable to either keep the original error or wrap the original error so that consumers can unwrap if necessary, i.e:

err = fmt.Errorf("no resolution %w", err)
mcdee commented 2 years ago

Thank you for reporting this issue. I have put together a potential fix for this, please take a look at the linked PR and let me know if you believe this should address it.

oldmanfleming commented 2 years ago

@mcdee thanks for getting back to me so quickly! This fix solves my issue perfectly.