Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors during fallback are no longer handled #48

Closed
pimterry opened this issue Oct 12, 2021 · 4 comments
Closed

Errors during fallback are no longer handled #48

pimterry opened this issue Oct 12, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@pimterry
Copy link
Contributor

In #43, it looks like we slightly changed the error handling behaviour. Repro:

const CacheableLookup = require('cacheable-lookup');

const cl = new CacheableLookup({
    lookup: (hostname, options, callback) => {
        setTimeout(() => {
            callback(Object.assign(new Error('Fake DNS error'), {
                code: 'EAI_AGAIN'
            }));
        }, 10);
    }
});

cl.lookup('example.test', (result) => {
    console.log('Got result', result);
});

In 6.0.1, this returns Error: cacheableLookup ENOTFOUND example.test
In 6.0.2, this returns Error: Fake DNS error

This affects any cases where normal resolution fails with ENOTFOUND or ENODATA, and then fallback fails with some other error. Previously we treated that as an ENOTFOUND result, now we use the fallback's error instead.

I think this comes from c5d06c5 (#43), which removed the try/catch inside _lookup.

I don't think this is necessarily wrong, but it's a change in behaviour that I don't think was intentional. What should happen in this case?

@szmarczak szmarczak added the bug Something isn't working label Oct 12, 2021
@szmarczak
Copy link
Owner

szmarczak commented Oct 12, 2021

This is a regression, will fix ASAP.

@szmarczak
Copy link
Owner

Well, to be honest we can either throw the lookup error or ENOTFOUND. The previous behavior was to ignore any lookup errors if real DNS queries were successful, which I'm not sure is the right behavior. Will do a release soon.

@szmarczak
Copy link
Owner

Released 6.0.3 🎉

@pimterry
Copy link
Contributor Author

⚡ amazingly fast fix, thanks! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants