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

Add sentry usage example #19

Merged
merged 1 commit into from
Mar 23, 2017
Merged

Conversation

kurayama
Copy link
Member

No description provided.

@kurayama kurayama self-assigned this Mar 23, 2017
@kurayama kurayama requested a review from ruimarinho March 23, 2017 12:12
README.md Outdated
@@ -51,7 +51,7 @@ processManager.once(async () => {

And it will now manage your node process.

### Loop
### loop()
Copy link
Member

Choose a reason for hiding this comment

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

loop(fn), on(fn), etc?

README.md Outdated
### [sentry.io](https://sentry.io)

The recommended way to report errors to sentry is by adding a `exit` hook and
sending each error using `captureException`.
Copy link
Member

Choose a reason for hiding this comment

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

Remove newline?

README.md Outdated
### [sentry.io](https://sentry.io)

The recommended way to report errors to sentry is by adding a `exit` hook and
sending each error using `captureException`.
Copy link
Member

Choose a reason for hiding this comment

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

Mention "(or captureExceptionAsync in case you promisify the method using a library like bluebird)"?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to return a promise, simply using captureException might not report the error as exit might run before the callback resolves.
I will revisit the wording.

README.md Outdated
const Promise = require('bluebird');
const raven = Promise.promisifyAll(require('raven'));

raven.config('https://******@sentry.io/<app id>').install();
Copy link
Member

Choose a reason for hiding this comment

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

appId?

README.md Outdated

processManager.addHook({
handler(errors) {
return Promise.map(errors, error => raven.captureExceptionAsync(error));
Copy link
Member

Choose a reason for hiding this comment

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

Did you end up revisiting - and confirming or rejecting - the map issue that fails all requests in case the first one fails?

Copy link
Member Author

Choose a reason for hiding this comment

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

If a promise fails during the mapping, the mapping stops and the overall promise rejected.
In this case, if there is a problem reporting a error to sentry (due to network, etc), probably the next ones will fail too (as they are running almost at the same time).

@kurayama kurayama force-pushed the support/improve-documentation branch 2 times, most recently from 8ab85a1 to b7213bd Compare March 23, 2017 15:32
README.md Outdated

### [sentry.io](https://sentry.io)

The recommended way to report errors to sentry is by adding a `exit` hook and sending each error using a promisified `captureException`.
Copy link
Contributor

Choose a reason for hiding this comment

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

an exit hook

README.md Outdated
@@ -110,7 +110,7 @@ processManager.once(function *() {
});
```

### Shutdown
### shutdown(fn)
Copy link
Member

Choose a reason for hiding this comment

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

{ error, force = false } = {}

@@ -92,7 +92,7 @@ function handler*(value) {
client.on('event', processManager.on(handler));
```

### Once
### once(fn)
Copy link
Member

Choose a reason for hiding this comment

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

Rename func to fn?

@kurayama kurayama force-pushed the support/improve-documentation branch 5 times, most recently from 3c967b5 to dab9d0d Compare March 23, 2017 16:32
README.md Outdated
- `args.handler` _(function)_: a function that returns a value or a thenable.
- `args.type` _(string)_: the hook type.
- `[args.name='a handler']` _(string)_: identifies the hook.
- `args.handler` _(function)_: a function that returns a value or a thenable.
Copy link
Member

Choose a reason for hiding this comment

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

Function?

@kurayama kurayama force-pushed the support/improve-documentation branch from dab9d0d to 75ff933 Compare March 23, 2017 16:44
@ruimarinho ruimarinho merged commit 240b16f into master Mar 23, 2017
@ruimarinho ruimarinho deleted the support/improve-documentation branch March 23, 2017 17:03
@kurayama kurayama mentioned this pull request Mar 23, 2017
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