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

Think of replacing/add throw error with reject #43

Open
alexisvisco opened this issue Jan 9, 2019 · 0 comments
Open

Think of replacing/add throw error with reject #43

alexisvisco opened this issue Jan 9, 2019 · 0 comments

Comments

@alexisvisco
Copy link

alexisvisco commented Jan 9, 2019

Hello,

I noticed that your code is ambiguous, indeed if we us promises we should not have to do a try catch. The code should, if it is in the promised mode, reject the error instead of the throwing it.

A sample of your code:

var whois = function whois(domain, opts, cb) {
  if (typeof domain !== 'string') {
    throw new Error('Expected a `domain name`');  //<< HERE
  }

  var callback;

  if (typeof opts === 'function') {
    callback = opts;
  } else {
    callback = cb;
  }

  if (_tldjs.default.tldExists(domain) === false) {
    throw new Error('Invalid a `domain name`'); //<< HERE
  }

  var encodedDomain = "".concat(encodePuny(domain)),
      promise = requestWhois(encodedDomain, opts);

  if (callback && typeof callback === 'function') {
    promise.then(function (data) {
      return callback(null, data);
    }).catch(function (error) {
      return callback(error, null);
    });
  } else {
    return promise;
  }
};

Thanks you!

@alexisvisco alexisvisco changed the title Did you know the existence of reject ? Think of replacing/add throw error with reject Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant