Stage errors for repo integration by cristiancavalli · Pull Request #2017 · googleapis/google-cloud-node

cristiancavalli · 2017-02-23T19:56:35Z

TODOS:

Refactor test directory to follow repo conventions
Unit and system tests pass
Fix linting errors
Switch from diagnostics-common to gcp-common

cristiancavalli · 2017-02-23T19:57:24Z

cc @GoogleCloudPlatform/node-team

ofrobots · 2017-02-23T20:19:10Z

This PR brings in the Stackdriver Error Reporting library as a client library. This is currently available as the @google/cloud-errors module. It would good to convert this to be the actual client library for Stackdriver error reporting (in the interest of minimizing the number of 'official' modules that work with Stackdriver error reporting). That is, we want to rename the module to be @google-cloud/error-reporting.

At the moment this only has support for the 'write' side of the Error Reporting API, but we expect to add the 'read' side of the API in follow-ons.

cristiancavalli · 2017-03-01T20:03:19Z

@GoogleCloudPlatform/node-team ready for a look

packages/error-reporting/.jshintrc

+  "undef": true,
+  "unused": "vars",
+  "globals": {
+    /* Mocha-provided globals */


packages/error-reporting/.travis.yml

+sudo: false
+language: node_js
+node_js:
+- '0.12'


packages/error-reporting/src/classes/custom-stack-trace.js

+ */
+CustomStackTrace.prototype.setFilePath = function(filePath) {
+
+  this.filePath = isString(filePath) ? filePath: '';


kjin

Looks good from my end (w/ nit).

packages/error-reporting/utils/fuzzer.js

+Fuzzer.prototype.generate.array = function(len, ofOneType, currentDepth, allowedDepth) {
+
+  var lenChecked = isNumber(len) ? len : random(1, 10);
+  var availableTypes = (isString(ofOneType) && (indexOf(this.types(), ofOneType) > -1)) ? [ofOneType]: this.types();


packages/error-reporting/.travis.yml

@@ -0,0 +1,8 @@
+sudo: false


packages/error-reporting/.npmignore

@@ -0,0 +1,5 @@
+/bin


packages/error-reporting/CHANGELOG.md

@@ -0,0 +1,29 @@
+# Node.js Agent for Google Cloud Errors ChangeLog


packages/error-reporting/bin/test.sh

@@ -0,0 +1,80 @@
+#!/usr/bin/env bash


packages/error-reporting/package.json

+  "main": "index.js",
+  "repository": "GoogleCloudPlatform/google-cloud-node",
+  "scripts": {
+    "test": "$(npm bin)/nyc --exclude=\"fuzzer.js\" $(npm bin)/mocha ./test/unit/*.js",


packages/error-reporting/package.json

@@ -0,0 +1,55 @@
+{
+  "name": "@google-cloud/error-reporting",
+  "version": "0.1.0",


packages/error-reporting/package.json

@@ -0,0 +1,55 @@
+{


packages/error-reporting/package.json

+  "scripts": {
+    "test": "$(npm bin)/nyc --exclude=\"fuzzer.js\" $(npm bin)/mocha ./test/unit/*.js",
+    "integration-tests": "$(npm bin)/nyc --exclude=\"error-message.js\" $(npm bin)/mocha ./test/system-test/*.js",
+    "style": "$(npm bin)/jshint lib index.js",


packages/error-reporting/package.json

+  "repository": "GoogleCloudPlatform/google-cloud-node",
+  "scripts": {
+    "test": "$(npm bin)/nyc --exclude=\"fuzzer.js\" $(npm bin)/mocha ./test/unit/*.js",
+    "integration-tests": "$(npm bin)/nyc --exclude=\"error-message.js\" $(npm bin)/mocha ./test/system-test/*.js",


packages/error-reporting/package.json

+    "@google-cloud/common": "^0.12.0",
+    "extend": "^3.0.0",
+    "is": "^3.2.0",
+    "lodash.has": "^4.5.2"


cristiancavalli · 2017-03-24T20:32:40Z

@stephenplusplus ready for another look

stephenplusplus · 2017-03-27T15:52:40Z

@cristiancavalli thanks! As far as the code of the library itself, I'm going to remain hands-off there and let you guys run it the way you see fit. As long as you're happy with how it looks and acts, I'm not concerned if it doesn't follow the conventions of the other parts of our library. I'm mostly concerned with the details of the integration when it comes to testing, releasing, and docs. Those are the types of questions I've brought up in comments above, attached to their relevant files in the PR. PTAL for those and feel free to elaborate on the expectations for these integrations.

packages/error-reporting/README.md

+> **This is not an official Google product.** This module is experimental and may not be ready for use.
+> This module uses APIs that may be undocumented and are subject to change without notice.
+
+This modules provides Stackdriver Error Reporting support for Node.js applications.


packages/error-reporting/README.md

+
+Container Engine nodes need to also be created with the `https://www.googleapis.com/auth/cloud-platform` scope, which is configurable during cluster creation. Alternatively, you can follow the instructions for using a service account under [running elsewhere](#running-elsewhere). It's recommended that you store the service account credentials as [Kubernetes Secret](http://kubernetes.io/v1.1/docs/user-guide/secrets.html).
+
+## Running elsewhere


packages/error-reporting/README.md

+```JS
+var express = require('express');
+var app = express();
+// Will create a errors instance based off env variables


packages/error-reporting/README.md

@@ -0,0 +1,230 @@
+# Node.js module for Stackdriver Error Reporting


packages/error-reporting/README.md

+  path: '/error',
+  handler: function(request, reply) {
+    throw new Error('Custom error message');
+    reply('Something broke!');


packages/error-reporting/package.json

+  "repository": "GoogleCloudPlatform/google-cloud-node",
+  "scripts": {
+    "test": "nyc --exclude=\"fuzzer.js\" $(npm bin)/mocha ./test/unit/*.js",
+    "system-test": "nyc --exclude=\"error-message.js\" $(npm bin)/mocha ./test/system-test/*.js",


packages/error-reporting/test/system-test/testAuthClient.js

@@ -0,0 +1,353 @@
+/**


packages/error-reporting/package.json

+    "test": "nyc --exclude=\"fuzzer.js\" $(npm bin)/mocha ./test/unit/*.js",
+    "system-test": "nyc --exclude=\"error-message.js\" $(npm bin)/mocha ./test/system-test/*.js",
+    "lint": "jshint src/ index.js",
+    "docs": "jsdoc -d docs index.js src/",


packages/error-reporting/package.json

+  "main": "index.js",
+  "repository": "GoogleCloudPlatform/google-cloud-node",
+  "scripts": {
+    "test": "nyc --exclude=\"fuzzer.js\" $(npm bin)/mocha ./test/unit/*.js",


packages/error-reporting/package.json

+    "@google-cloud/common": "^0.12.0",
+    "extend": "^3.0.0",
+    "is": "^3.2.0",
+    "lodash.has": "^4.5.2"


stephenplusplus · 2017-03-30T17:05:39Z

Moving the convo about how we're planning to integrate out here instead of in the code comments section

RE: #2017 (comment)

Let me know if this sounds right. Before we merge, this is the extent of our process integration.

The GCN CI:

will not include error-reporting in our coverage report for the repo
will not run error-reporting unit tests
will not run error-reporting system tests
will include error-reporting docs on https://googlecloudplatform.github.io/google-cloud-node/#/

cristiancavalli · 2017-03-30T17:52:38Z

@stephenplusplus Your proposal seems like the best given the current constraints

stephenplusplus · 2017-03-30T17:53:25Z

Cool, then @callmehiphop, can you step in and figure out what we need to do for docs?

Also cc @lukesneeringer for any thoughts on the "big picture" plan outlined above.

ofrobots · 2017-03-30T18:15:30Z

For the purposes of making progress on this PR, it makes sense to skip CI for now, but I would expect a follow-on refactor to add the unit tests and system tests back into the CI. At least that is my understanding of the integration here.

lukesneeringer · 2017-03-31T14:31:08Z

For the purposes of making progress on this PR, it makes sense to skip CI for now, but I would expect a follow-on refactor to add the unit tests and system tests back into the CI. At least that is my understanding of the integration here.

That makes me all kinds of nervous. Is there a hard deliverable or OKR to get this done?

What is the challenge here? Is it just moving the tests up one level, or something else?

ofrobots · 2017-03-31T15:17:41Z

@lukesneeringer The idea was to keep the review burden on y'all for the initial PR low. If that is not a problem, then we can add the CI refactoring here too. I am not too worried though as the origin repo for this code is still running CI, and my expectation is that we will immediately follow-up on with CI support this repo once this PR lands.

lukesneeringer · 2017-03-31T15:18:32Z

@ofrobots Sold.

ofrobots · 2017-03-31T16:08:50Z

I am guessing that the 'sold' refers to landing + following up with a PR to enable CI support. @cristiancavalli, @stephenplusplus anything else that remains to be done before proceeding here?

cristiancavalli · 2017-03-31T16:10:46Z

@ofrobots 👍

lukesneeringer · 2017-03-31T18:22:55Z

@ofrobots It essentially meant, "I accept what you said and am happy with it." Sorry. :-)

stephenplusplus · 2017-03-31T18:24:55Z

I believe we still need to figure out the docs, which is why this is assigned to @callmehiphop.

callmehiphop

For a documentation review, we need a few things

Overview configs

Essentially I think we just need the following code added to the OVERVIEW map

    'error-reporting': {
      title: 'Error Reporting',
      instanceName: 'errors'
    },

Otherwise I found the examples in the index.js file to be kind of lacking? We don't quite follow the ways of JSDoc, so this would be the only file to really show up on our doc site.

A few alterations are also needed within our documentation scripts, but I can put a PR together for that this afternoon.

packages/error-reporting/src/configuration.js

+ * @memberof Configuration
+ * @public
+ * @function getCredentials
+ * @returns {Credentials\Null} - returns the _credentials property


packages/error-reporting/index.js

@@ -0,0 +1,97 @@
+/**


packages/error-reporting/index.js

+ * this function will also return an interface which can be used manually via
+ * the `report` function property, with hapi via the `hapi` object property or
+ * with express via the `express` function property.
+ * @function initConfiguration


packages/error-reporting/index.js

+  uncaughtException(client, config);
+
+  // Build the application interfaces for use by the hosting application
+  this.hapi = hapi(client, config);


stephenplusplus · 2017-04-03T20:10:28Z

Adding onto the comment about examples in index.js, is it possible to move this file to the src directory?

cristiancavalli · 2017-04-04T18:36:24Z

@stephenplusplus @callmehiphop Thanks for the feedback gentlemen! Changes are posted

cc @ofrobots

cristiancavalli · 2017-04-04T20:09:30Z

@stephenplusplus freshly rebased

stephenplusplus · 2017-04-04T22:25:37Z

Looks like the lint task is failing. Can you run 'npm run lint' from the root directory to take a look?

cristiancavalli · 2017-04-04T22:38:41Z

@stephenplusplus updated

cristiancavalli · 2017-04-05T16:13:21Z

@stephenplusplus @callmehiphop

I'm having some trouble with the snippet test, specifically this error from the ci:

Cannot find module '../packages/error-reporting/node_modules/@google-cloud/error-reporting

Would you gentlemen know where this is coming from and how to fix it?

ofrobots · 2017-04-06T15:14:31Z

@stephenplusplus @lukesneeringer bump. It would be good to be able to land this soon.

callmehiphop · 2017-04-06T15:44:54Z

@cristiancavalli sorry I didn't catch this one in my docs review! When we write examples we don't include any snippets on how to require the module itself. This code usually is generated and included within an overview. After which, we assume any requiring of @google-cloud packages is showing how to integrate with other modules - so we do some magic to require the local instance of whatever package.

Long story short, the fix would be to remove the require('@google-cloud/error-reporting') parts of the examples from the docs.

cristiancavalli · 2017-04-06T16:17:32Z

@callmehiphop Thanks for the info! Updated the PR and added fake injection dependencies for snippet tests.

coveralls · 2017-04-06T16:28:21Z

Changes Unknown when pulling 475eedb on cristiancavalli:error-library-integration into ** on GoogleCloudPlatform:master**.

cristiancavalli · 2017-04-06T17:15:30Z

Build is now passing on CircleCI

cc @stephenplusplus @ofrobots

stephenplusplus · 2017-04-06T17:20:50Z

Hooray!! 🎉 🍰

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 23, 2017

cristiancavalli requested review from ofrobots and stephenplusplus March 1, 2017 20:03

kjin reviewed Mar 6, 2017

View reviewed changes

kjin approved these changes Mar 10, 2017

View reviewed changes

stephenplusplus suggested changes Mar 23, 2017

View reviewed changes

stephenplusplus added the api: error-reporting label Mar 23, 2017

stephenplusplus suggested changes Mar 30, 2017

View reviewed changes

stephenplusplus assigned callmehiphop Mar 30, 2017

callmehiphop suggested changes Apr 3, 2017

View reviewed changes

callmehiphop mentioned this pull request Apr 3, 2017

docs: update script parser for error-reporting #2181

Merged

callmehiphop removed their assignment Apr 3, 2017

cristiancavalli and others added 8 commits April 4, 2017 13:05

remove 0.12 node runtime from travis.yml

e2f0ed9

fix ternary statement in fuzzer

4785e7a

rename module; README tweaks

9ca6f90

start addressing pr feedback (round 2)

217c3a5

address pr feedback

b27025f

pr feedback, fix issue with system tests

245563f

fix spacing on README

fbf933d

address documentation feedback, move index into ./src/

58af011

cristiancavalli force-pushed the error-library-integration branch from df8c167 to 58af011 Compare April 4, 2017 20:08

fix lint

ace6a15

cristiancavalli added 2 commits April 5, 2017 08:40

change function syntax in examples because jshint is complianing

f3f6134

fix snippet test

a403a34

cristiancavalli added 2 commits April 6, 2017 08:48

remove errors requires from example comments

4485f5e

fix snippet tests; add dependencies for errors snippet tests.

475eedb

stephenplusplus merged commit 4fc204c into googleapis:master Apr 6, 2017

GautamSharda pushed a commit that referenced this pull request Jan 14, 2026

Stage errors for repo integration (#2017)

8db86e1

GautamSharda pushed a commit that referenced this pull request Jan 15, 2026

Stage errors for repo integration (#2017)

1bf4798

GautamSharda mentioned this pull request Jan 20, 2026

chore: migrate code from googleapis/nodejs-error-reporting #7019

Closed

4 tasks

GautamSharda pushed a commit that referenced this pull request Feb 5, 2026

Stage errors for repo integration (#2017)

4e112ae

GautamSharda mentioned this pull request Feb 5, 2026

migrate code from googleapis/nodejs-error-reporting #7169

Open

4 tasks

		@@ -0,0 +1,29 @@
		# Node.js Agent for Google Cloud Errors ChangeLog


		Container Engine nodes need to also be created with the `https://www.googleapis.com/auth/cloud-platform` scope, which is configurable during cluster creation. Alternatively, you can follow the instructions for using a service account under [running elsewhere](#running-elsewhere). It's recommended that you store the service account credentials as [Kubernetes Secret](http://kubernetes.io/v1.1/docs/user-guide/secrets.html).

		## Running elsewhere

		@@ -0,0 +1,230 @@
		# Node.js module for Stackdriver Error Reporting

Conversation

cristiancavalli commented Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristiancavalli commented Feb 23, 2017

Uh oh!

ofrobots commented Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristiancavalli commented Mar 1, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

kjin left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

cristiancavalli commented Mar 24, 2017

Uh oh!

stephenplusplus commented Mar 27, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

cristiancavalli commented Feb 23, 2017 •

edited

Loading

ofrobots commented Feb 23, 2017 •

edited

Loading