Skip to content

Handles hooks load error #3244

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

Conversation

yuki-takeichi
Copy link
Contributor

I changed the behavior when Parse Server fails to load _Hooks collection to prevent it from launching with incomplete state. If Parse Server fails to load _Hooks entries, currently it ignores error and launches without setting hooks. This behavior causes serious problems. (For example an important validation logic doesn't work in Before Save Hook.) So my change forces Parse Server to shutdown before it accepts requests.

@yuki-takeichi
Copy link
Contributor Author

Feel free to revise my English in source code. I'm not native English speaker.

@@ -243,7 +243,6 @@ class ParseServer {
Config.validate(AppCache.get(appId));
this.config = AppCache.get(appId);
Config.setupPasswordValidator(this.config.passwordPolicy);
hooksController.load();
Copy link
Contributor

Choose a reason for hiding this comment

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

This should stay here

Copy link
Contributor

Choose a reason for hiding this comment

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

And you validation logic should throw if there's an error loading the hooks, inside the hooks controller.

const server = app.listen(options.port, options.host, callback);
server.on('connection', initializeConnections);
const hooksController = AppCache.get(options.appId)['hooksController'];
hooksController.load().then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

No modifications in that file please. Some people don't use the CLIz

@facebook-github-bot
Copy link

@yuki-takeichi do you have any updates for this pull request? It's been a while since the last update so wanted to check in and see if you've looked at the requested changes.

@flovilmart
Copy link
Contributor

@yuki-takeichi I'll be closing this one now, feel free to reopen when the addressing the conflicts and requests.

@flovilmart flovilmart closed this Mar 10, 2018
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

Successfully merging this pull request may close these issues.

3 participants