Skip to content

Remove uncaught interface #2208

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.

Sign up for GitHub

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

Jump to bottom
Merged
stephenplusplus merged 1 commit into from
Apr 14, 2017
Merged

Remove uncaught interface #2208

stephenplusplus merged 1 commit into from
Apr 14, 2017

Conversation

@cristiancavalli
Copy link
Contributor

@cristiancavalli cristiancavalli commented Apr 11, 2017 •
edited
Loading

Blockers

Remove Uncaught Exception Handler

  • Remove the uncaught exception handler
  • Remove the corresponding tests & documentation
  • Add new documentation to README stating how feature can be recreated in application code

Note:

Commit is currently larger than actual landing-scope since it is stacked on in-flight PR #2189

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 11, 2017
@coveralls
Copy link

coveralls commented Apr 11, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 8286972 on cristiancavalli:remove-uncaught-interface into 87312f9 on GoogleCloudPlatform:master.

ofrobots
ofrobots suggested changes Apr 11, 2017
View reviewed changes
Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

I think the README.md changes are missing.

@cristiancavalli cristiancavalli force-pushed the remove-uncaught-interface branch from 8286972 to ccd0553 Compare April 11, 2017 18:23
@cristiancavalli
Copy link
Contributor Author

cristiancavalli commented Apr 11, 2017

@ofrobots my bad -- weird rebase flow.

Changes added

@cristiancavalli cristiancavalli force-pushed the remove-uncaught-interface branch from ccd0553 to ec21135 Compare April 11, 2017 18:25
ofrobots
ofrobots reviewed Apr 11, 2017
View reviewed changes
packages/error-reporting/README.md Outdated
errors.report(new Error('Something broke!'));
```

- *One may even catch and report application-wide uncaught errors:*

This comment was marked as spam.

Sign in to view

packages/error-reporting/README.md Outdated

```js
var errors = require('@google-cloud/error-reporting')();
process.on('uncaughtException', e => errors.report(e));

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@coveralls
Copy link

coveralls commented Apr 11, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling ec21135 on cristiancavalli:remove-uncaught-interface into 87312f9 on GoogleCloudPlatform:master.

@cristiancavalli cristiancavalli force-pushed the remove-uncaught-interface branch from bfe989f to fb23ced Compare April 11, 2017 18:57
@cristiancavalli
Copy link
Contributor Author

cristiancavalli commented Apr 11, 2017

@ofrobots updated

@coveralls
Copy link

coveralls commented Apr 11, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling fb23ced on cristiancavalli:remove-uncaught-interface into 87312f9 on GoogleCloudPlatform:master.

@stephenplusplus
Copy link
Contributor

stephenplusplus commented Apr 11, 2017

@ofrobots let me know when you're happy with this and I'll try to merge within 10 seconds.

ofrobots
ofrobots reviewed Apr 11, 2017
View reviewed changes
packages/error-reporting/README.md Outdated
process.on('uncaughtException', e => errors.report(e));
process.on('uncaughtException', (e) => {
// Write the error to stderr. If one does not manually do this
// nothing will be printed before the VM exits.

This comment was marked as spam.

Sign in to view

ofrobots
ofrobots reviewed Apr 11, 2017
View reviewed changes
packages/error-reporting/README.md Outdated
- *One may even catch and report application-wide uncaught errors:*
- **One may even catch and report application-wide uncaught errors:**
- *It is recommended to catch uncaughtExceptions for production-deployed applications*
- [To read more about uncaught exception handling in Node.js and what it means for your application please click here](https://nodejs.org/api/process.html#process_event_uncaughtexception)

This comment was marked as spam.

Sign in to view

@cristiancavalli cristiancavalli force-pushed the remove-uncaught-interface branch from fb23ced to 9116a94 Compare April 11, 2017 20:39
@cristiancavalli
Copy link
Contributor Author

cristiancavalli commented Apr 11, 2017

@ofrobots updated

ofrobots
ofrobots approved these changes Apr 11, 2017
View reviewed changes
Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

LGTM. Is this blocked on #2189?

@cristiancavalli
Copy link
Contributor Author

cristiancavalli commented Apr 11, 2017

@ofrobots yes these commits are stacked on top of #2189

@coveralls
Copy link

coveralls commented Apr 11, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 9116a94 on cristiancavalli:remove-uncaught-interface into 184f9bb on GoogleCloudPlatform:master.

@ofrobots ofrobots mentioned this pull request Apr 12, 2017
Merged
6 tasks
@stephenplusplus stephenplusplus added api: error-reporting status: blocked Resolving the issue is dependent on other work. labels Apr 13, 2017
@stephenplusplus
Copy link
Contributor

stephenplusplus commented Apr 13, 2017

Is this really over?

@cristiancavalli
Copy link
Contributor Author

cristiancavalli commented Apr 13, 2017

No not yet, sorry was trying to see if closing this had an effect on the PR file propagation for the CI integration

@cristiancavalli cristiancavalli force-pushed the remove-uncaught-interface branch from 9116a94 to 6c1e4b4 Compare April 13, 2017 20:57
@cristiancavalli
Copy link
Contributor Author

cristiancavalli commented Apr 13, 2017

@stephenplusplus @ofrobots merge conflicts resolved!

@coveralls
Copy link

coveralls commented Apr 13, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 6c1e4b4 on cristiancavalli:remove-uncaught-interface into f0812b6 on GoogleCloudPlatform:master.

@cristiancavalli cristiancavalli force-pushed the remove-uncaught-interface branch from 6c1e4b4 to 54a6ead Compare April 13, 2017 21:21
@coveralls
Copy link

coveralls commented Apr 13, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 54a6ead on cristiancavalli:remove-uncaught-interface into f0812b6 on GoogleCloudPlatform:master.

@stephenplusplus stephenplusplus removed the status: blocked Resolving the issue is dependent on other work. label Apr 14, 2017
@stephenplusplus stephenplusplus merged commit 326ad28 into googleapis:master Apr 14, 2017
GautamSharda pushed a commit that referenced this pull request Jan 14, 2026
@cristiancavalli @stephenplusplus
remove uncaught interface (#2208)
88023cd
GautamSharda pushed a commit that referenced this pull request Jan 15, 2026
@cristiancavalli @stephenplusplus
remove uncaught interface (#2208)
28666d5
@GautamSharda GautamSharda mentioned this pull request Jan 20, 2026
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@ofrobots ofrobots ofrobots approved these changes

@stephenplusplus stephenplusplus stephenplusplus approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: error-reporting cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@cristiancavalli @coveralls @stephenplusplus @ofrobots @googlebot