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

Error message for invalid auth provider #528

Merged
merged 4 commits into from
May 23, 2019

Conversation

maxmarkus
Copy link
Contributor

Fixes #510

@maxmarkus maxmarkus added bug Something isn't working area/luigi labels May 15, 2019
@maxmarkus maxmarkus added this to the Sprint_Swinka_13 milestone May 15, 2019
@hardl hardl removed the area/luigi label May 15, 2019
@jesusreal jesusreal self-assigned this May 20, 2019
Copy link
Contributor

@jesusreal jesusreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comments below

`Could not instantate ${idpProviderName} provider : ${err}`
);
const errorMsg = `Error: ${err.message || err}`;
console.error(errorMsg, err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be just console.error(errorMsg);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is inside an if block:

      if (GenericHelpers.isPromise(idpProviderInstance)) {

Looks like we will not display the error if the idpProviderInstance is not a promise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the implementation to output the second err (object) only in case there is a message found.
If the Provider does not return a Promise, it is up to the provider itself to throw any required error.

core/src/services/config.js Outdated Show resolved Hide resolved
@JohannesDoberer JohannesDoberer self-assigned this May 21, 2019
@maxmarkus maxmarkus merged commit 3a5d09a into SAP:master May 23, 2019
@maxmarkus maxmarkus deleted the 510-fix-invalid-provider-error branch May 23, 2019 09:44
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
* simplified auth check, added error message
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

Successfully merging this pull request may close these issues.

Invalid auth provider should throw an error
4 participants