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

log DB-connection error #2047

Closed
wants to merge 1 commit into from
Closed

log DB-connection error #2047

wants to merge 1 commit into from

Conversation

lc-thomasberger
Copy link
Member

fixes #880

Unfortunately we can only use lib/database once the app is started (preload the configuration module). Therefore we can't check potential connection issues earlier in the install script.

This PR emit's an error event when the app can't be started. This is mostly related to not being able to connect to the DB. The install script displays this error message to the user.
This makes the overall installation experience a bit better, as it gives useful information about what caused the error.

@lc-thomasberger lc-thomasberger added the S: awaiting-review Completed issues waiting on reviews label Aug 14, 2018
@lc-thomasberger lc-thomasberger self-assigned this Aug 14, 2018
@lc-thomasberger lc-thomasberger added this to the 0.5.1 milestone Aug 14, 2018
Copy link
Contributor

@tomgreenfield tomgreenfield left a comment

Choose a reason for hiding this comment

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

👁

@@ -417,6 +417,9 @@ function configureMasterTenant(callback) {
installHelpers.showSpinner('Starting server');
// run the app
app.run({ skipVersionCheck: true });
app.on('serverStartError', function(error) {
return exit(1, error.message || error);
});
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this just replicates the error handling in lib/application.js

Copy link
Member Author

Choose a reason for hiding this comment

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

It also logs the error message that connection failed.
Without that I don't get this message.

@lc-thomasberger
Copy link
Member Author

Is this still needed in regards to #2059

@taylortom taylortom closed this Sep 3, 2018
@taylortom taylortom deleted the issue/880-2 branch September 4, 2018 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: awaiting-review Completed issues waiting on reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants