Skip to content

Commit c9cef5a

Browse files
authored
fix: do not try and authenticate when error reporting is disabled (#676)
* fix: do not try and authenticate when error reporting is disabled * remove extra commit * lint * Update test to check auth call * fix lint * unnock
1 parent 25a65d9 commit c9cef5a

File tree

3 files changed

+31
-4
lines changed

3 files changed

+31
-4
lines changed

src/google-apis/auth-client.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,11 @@ export class RequestHandler extends Service {
110110
this._config = config;
111111
this._logger = logger;
112112

113-
if (tryAuthenticate) {
113+
if (!this._config.getShouldReportErrorsToAPI()) {
114+
this._logger.info(
115+
'Not configured to send errors to the API; skipping Google Cloud API Authentication.'
116+
);
117+
} else if (tryAuthenticate) {
114118
this.authClient.getAccessToken().then(
115119
() => {},
116120
err => {

system-test/error-reporting.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,15 +383,21 @@ describe('Expected Behavior', () => {
383383
env.sterilizeProcess();
384384
});
385385

386-
it('Should callback with an error with a configuration that cannot report errors', done => {
386+
it('Should not call auth and should callback with an error in a configuration that cannot report errors', done => {
387387
env.sterilizeProcess().setKeyFilename().setProjectId();
388+
const scope = nock('https://www.googleapis.com:443')
389+
.post('/oauth2/v4/token')
390+
.reply(400);
388391
process.env.NODE_ENV = 'null';
389392
const logger = createLogger({logLevel: 5, reportMode: 'production'});
390393
const client = new RequestHandler(
391394
new Configuration(undefined, logger),
392395
logger
393396
);
397+
394398
client.sendError({} as ErrorMessage, (err, response) => {
399+
assert.strictEqual(scope.isDone(), false);
400+
nock.cleanAll();
395401
assert(err instanceof Error);
396402
assert.strictEqual(err!.message, ERROR_STRING);
397403
assert.strictEqual(response, null);

test/unit/google-apis/auth-client.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,25 @@ describe('RequestHandler', () => {
110110
);
111111
});
112112

113+
it('should not request OAuth2 token if error reporting is disabled', done => {
114+
verifyReportedMessage(
115+
{reportMode: 'never'},
116+
null, // no access token error
117+
{
118+
info: 'Not configured to send errors to the API; skipping Google Cloud API Authentication.',
119+
},
120+
done
121+
);
122+
});
123+
113124
it('should not issue a warning if disabled and can communicate with the API', done => {
114125
process.env.NODE_ENV = 'production';
115126
verifyReportedMessage(
116127
{reportMode: 'never'},
117128
null, // no access token error
118-
{}, // no expected logs
129+
{
130+
info: 'Not configured to send errors to the API; skipping Google Cloud API Authentication.',
131+
},
119132
done
120133
);
121134
});
@@ -125,7 +138,9 @@ describe('RequestHandler', () => {
125138
verifyReportedMessage(
126139
{reportMode: 'never'},
127140
null, // no access token error
128-
{}, // no expected logs
141+
{
142+
info: 'Not configured to send errors to the API; skipping Google Cloud API Authentication.',
143+
},
129144
done
130145
);
131146
});
@@ -166,6 +181,7 @@ describe('RequestHandler', () => {
166181
{reportMode: 'production'},
167182
null, // no access token error
168183
{
184+
info: 'Not configured to send errors to the API; skipping Google Cloud API Authentication.',
169185
warn:
170186
'The error reporting client is configured to report ' +
171187
'errors if and only if the NODE_ENV environment variable is set to ' +
@@ -183,6 +199,7 @@ describe('RequestHandler', () => {
183199
{},
184200
null, // no access token error
185201
{
202+
info: 'Not configured to send errors to the API; skipping Google Cloud API Authentication.',
186203
warn:
187204
'The error reporting client is configured to report ' +
188205
'errors if and only if the NODE_ENV environment variable is set to ' +

0 commit comments

Comments
 (0)