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

[ftr] log async init errors right when they happen #11898

Merged
merged 1 commit into from
May 22, 2017

Conversation

spalger
Copy link
Contributor

@spalger spalger commented May 18, 2017

Rather than just throwing the first error that happens during async provider loading, log all errors as they occur and then throw an error at the end mentioning all of the services that failed, by name.

@spalger spalger added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc test v5.5.0 v6.0.0 labels May 18, 2017
@spalger spalger requested a review from rhoboat May 18, 2017 15:04
@spalger spalger force-pushed the ftr/log-async-init-errors branch from f45a1f1 to 1fcf774 Compare May 18, 2017 15:04
@spalger spalger force-pushed the ftr/log-async-init-errors branch from 1fcf774 to 6f2f274 Compare May 18, 2017 20:57
Copy link

@rhoboat rhoboat left a comment

Choose a reason for hiding this comment

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

lgtm! 📗

@@ -35,16 +37,15 @@ export class ProviderCollection {
await instance.init();
}
} catch (err) {
asyncInitErrors.push(err);
this._log.warning('Failure loading service %j', name);
this._log.error(err);
Copy link

Choose a reason for hiding this comment

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

👍

@@ -2,7 +2,8 @@ import { loadTracer } from '../load_tracer';
import { createAsyncInstance, isAsyncInstance } from './async_instance';

export class ProviderCollection {
constructor(providers) {
constructor(log, providers) {
this._log = log;
this._instances = new Map();
this._providers = providers;
Copy link

Choose a reason for hiding this comment

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

the _private naming thing is a thing we do? is the point just to indicate to others that this is meant to be used in a private way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

@spalger spalger merged commit 192b2ea into elastic:master May 22, 2017
spalger added a commit that referenced this pull request May 22, 2017
@spalger
Copy link
Contributor Author

spalger commented May 22, 2017

5.5/5.x: 595294e

snide pushed a commit to snide/kibana that referenced this pull request May 30, 2017
@spalger spalger deleted the ftr/log-async-init-errors branch October 18, 2019 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc test v5.5.0 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants