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 Check for Invalid iKey #1243

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Add Check for Invalid iKey #1243

merged 1 commit into from
Nov 15, 2023

Conversation

JacksonWeber
Copy link
Contributor

If an invalid iKey is passed to application insights we should disable statsbeat.

Copy link
Member

@hectorhdzg hectorhdzg left a comment

Choose a reason for hiding this comment

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

LGTM

@JacksonWeber JacksonWeber merged commit 2b142c8 into microsoft:develop Nov 15, 2023
8 checks passed
@d00rsfan
Copy link

d00rsfan commented Jan 16, 2024

It began to throw exception!
Which is i think wrong behaviour for minor version change (2.9.1->2.9.2), if exception not catched, app will die!
Error: Instrumentation key not found, please provide a connection string before starting Application Insights SDK. at NodeClient.TelemetryClient

@JacksonWeber
Copy link
Contributor Author

It began to throw exception! Which is i think wrong behaviour for minor version change (2.9.1->2.9.2), if exception not catched, app will die! Error: Instrumentation key not found, please provide a connection string before starting Application Insights SDK. at NodeClient.TelemetryClient

Can I see what you're using to initialize the SDK on your side? I don't believe this change should be throwing that error for you, but happy to take a look.

@d00rsfan
Copy link

Thank you for reply!
I have reproduced it. Was confused that uncaught exception happened after 70sec of uptime.
No reason to create project. It has only 1 file and 1 dependency in package.json:
This programm will die in 70sec

import appInsights from 'applicationinsights';

appInsights.setup('foobar').start();

let sec = 10;
setInterval(() => {
  console.log(`uptime=${sec}seconds`);
  sec+=10;
}, 10000);
console.log('started');

.........
uptime=70seconds
ApplicationInsights:Instrumentation key was invalid, please check the iKey []
/path/to/thisproject/node_modules/applicationinsights/out/Library/Sender.js:199
_this._shutdownStatsbeat();
^

TypeError: _this._shutdownStatsbeat is not a function
at IncomingMessage. (/path/to/thisproject/node_modules/applicationinsights/out/Library/Sender.js:199:47)
at IncomingMessage.emit (node:events:530:35)
at endReadableNT (node:internal/streams/readable:1696:12)
at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

Node.js v20.11.0

@d00rsfan
Copy link

If version locked to "applicationinsights": "2.9.1", this example works forever.

@JacksonWeber
Copy link
Contributor Author

Thank you for reply! I have reproduced it. Was confused that uncaught exception happened after 70sec of uptime. No reason to create project. It has only 1 file and 1 dependency in package.json: This programm will die in 70sec

import appInsights from 'applicationinsights';

appInsights.setup('foobar').start();

let sec = 10;
setInterval(() => {
  console.log(`uptime=${sec}seconds`);
  sec+=10;
}, 10000);
console.log('started');

......... uptime=70seconds ApplicationInsights:Instrumentation key was invalid, please check the iKey [] /path/to/thisproject/node_modules/applicationinsights/out/Library/Sender.js:199 _this._shutdownStatsbeat(); ^

TypeError: _this._shutdownStatsbeat is not a function at IncomingMessage. (/path/to/thisproject/node_modules/applicationinsights/out/Library/Sender.js:199:47) at IncomingMessage.emit (node:events:530:35) at endReadableNT (node:internal/streams/readable:1696:12) at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

Node.js v20.11.0

I see. #1259 should address this issue and will be included in the next release.

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