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

DRAFT: Proposed error improvements #97

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robertjd
Copy link
Contributor

@robertjd robertjd commented Apr 2, 2019

There are two common configuration errors that developers run into:

  • Invalid API token
  • Invalid Org URL (DNS name not found)

In either situation, the underlying HTTP error bubbles up the promise chain.

If the developer has not implemented a catch, they get a generic error about an uncaught promise exception

This improves on that by printing the exceptions for the common cases above, then exiting the process with a non-zero code

You can reproduce the problematic case by running this:

const okta = require('@okta/okta-sdk-nodejs');

const client = new okta.Client({
  orgUrl: 'https://fooasdoasdfj.okta-asdofij.com',
  token: 'foo',
  requestExecutor: new okta.DefaultRequestExecutor()
});


client.listUsers().each(user => {
  console.log(user)
}).then(() => {
  console.log("Listed users.");
  process.exit();
})

There are two common configuration errors that developers run into:
- Invalid API token
- Invalid Org URL (DNS name not found)

In either situation, the underlying HTTP error bubbles up the promise chain.

If the developer has not implemented a catch, they get a generic error about an uncaught promise exception

This improves on that by printing the exceptions for the common cases above, then exiting the process with a non-zero code
@aarongranick-okta
Copy link
Contributor

I definitely support logging the error. Killing the process may be a bit severe. The Okta integration could be a small part of a much larger application.

@robertjd
Copy link
Contributor Author

robertjd commented Apr 2, 2019

I definitely support logging the error. Killing the process may be a bit severe. The Okta integration could be a small part of a much larger application.

That's a good point, we shouldn't exit. I was doing the exit to try to avoid the "UnhandledPromiseRejectionWarning" also being printed, but maybe there's not a good way to do that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants