Skip to content
This repository has been archived by the owner on Aug 6, 2021. It is now read-only.

Update Lifecyclecallbacks.md with error handling #853

Merged
merged 1 commit into from Nov 16, 2017

Conversation

ashleysimons
Copy link
Contributor

Clear advice about making sure thrown errors or rejections make it back to the callback and why.

Clear advice about making sure thrown errors or rejections make it back to the callback and why.
@sgress454
Copy link
Member

LGTM!

@sgress454 sgress454 merged commit 37ab443 into balderdashy:master Nov 16, 2017
@sgress454
Copy link
Member

Thanks @ashleysimons !

Copy link
Member

@mikermcneil mikermcneil left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to write this up! A few asks before merging


It's important to realize that Sails will not catch thrown errors in your callbacks. If there is any chance the code you are executing will throw an `Error`, please wrap it in a `try catch` and pass the error to the callback.
Copy link
Member

Choose a reason for hiding this comment

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

Can we clarify "in lifecycle callbacks" here?

@@ -67,6 +67,33 @@ module.exports = {
};
```

### Error handling ###
Copy link
Member

Choose a reason for hiding this comment

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

Can we trim the extra set of ###- it could confuse our doctemplater tool

}
```

If you are using promises, please define a `catch` and pass the error to the callback. A promise will swallow the error.
Copy link
Member

Choose a reason for hiding this comment

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

Can we replace this with an async/await example?

@mikermcneil
Copy link
Member

@ashleysimons Reverted in #936 but would love to remerge if you wouldn't mind making the changes above

Also, would you make the changes against the 1.0 branch?

Thanks again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
3 participants