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

[BUG] SDK LOAD Failure reporting not working #2027

Closed
MichelJansson opened this issue Apr 1, 2023 · 2 comments
Closed

[BUG] SDK LOAD Failure reporting not working #2027

MichelJansson opened this issue Apr 1, 2023 · 2 comments
Assignees
Labels
released - NPM snippet Snippet update required or suggested
Milestone

Comments

@MichelJansson
Copy link

Description/Screenshot
SDK LOAD Failure is not being sent successfully with instrumentation key errors as response.
This is all due to a mix of issues in the actual snippet code but also the snippet as presented in the README: see additional context below.

Steps to Reproduce

  1. Use latest snippet from README
  2. Configure using connection string as copied from Azure portal:
    connectionString: "InstrumentationKey=[KEY];IngestionEndpoint=[ENDPOINT];LiveEndpoint=[LIVEENDPOINT]"
  3. Enter an invalid URL as script src to simulate loading error
  4. Load page and inspect track request.
    • Notice iKey of payload containing the entire connection string and not just the instrumentation key.
    • Notice the http 400 response with "Invalid instrumentation key" message.

Expected behavior
Connection string to be parsed correctly and exception being tracked successfully.

Additional context

  • Using connection string only has not been working for quite some time due to a mistake in the snippet:

    1. The connection string is being parsed and it's parts is being stored as lower case in the snippet
      var kvPairs = connectionString.split(";");
      for (var lp = 0; lp < kvPairs.length; lp++) {
      var kvParts = kvPairs[lp].split("=");
      if (kvParts.length === 2) { // only save fields with valid formats
      fields[kvParts[0][strToLowerCase]()] = kvParts[1];
      }
      }
    2. But then when the load error code tries to get the instrumentation key from the connection string, it uses a constant that is not lower case which results in no instrumentation key being found.
      var conString = _parseConnectionString();
      var iKey = conString[strInstrumentationKey] || aiConfig[strInstrumentationKey] || strEmpty;
  • The above issue used to possible to be worked around by also providing an instrumenctionKey in the snippet config that then would be used instead, but with the snippet from the current readme this no longer works due:

    1. Someone updated the readme to remove instrumentation key config, and I suppose accidentally also replaced the constant D in the snippet from instrumentationKey to connectionString.
    2. This leads to the entire connection string being passed as iKey as noted of the repro steps.

I am willing to post a PR if you are open for that.

@MSNev
Copy link
Collaborator

MSNev commented Apr 3, 2023

I am willing to post a PR if you are open for that.

Yep, go for it

@MSNev MSNev self-assigned this Apr 6, 2023
@MSNev MSNev added this to the 2.8.12 milestone Apr 6, 2023
@MSNev MSNev added the snippet Snippet update required or suggested label Apr 6, 2023
@MSNev MSNev added the fixed - waiting release PR Committed and waiting deployment label Apr 10, 2023
@MSNev MSNev added waiting - CDN deployment released - NPM and removed fixed - waiting release PR Committed and waiting deployment labels Apr 11, 2023
MSNev added a commit that referenced this issue Apr 12, 2023
* [BUG] The documentation for enableDebug is incorrect, it should reference enableDebugExceptions #2014 (#2022)

* [BUG] SDK LOAD Failure reporting not working #2027 (#2038)

* Add a simple interface to enable custom buffer storage solutions #1419 (#2037)

This can be used like:

```
const appInsights = new ApplicationInsights({
  config: {
    enableSessionStorageBuffer: true,
    bufferOverride: {
      getItem: (logger, key) => localStorage.getItem(key),
      setItem: (logger, key, value) => localStorage.setItem(key, value),
    }
  }
});
```

Co-authored-by: Nev <54870357+MSNev@users.noreply.github.com>

* [Master] Add readme documentation for IStorageBuffer (#2045)

* [Release] Increase version to 2.8.12 (#2046)

---------

Co-authored-by: Philip Peitsch <philip.peitsch@gmail.com>
@MSNev MSNev closed this as completed Apr 20, 2023
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
released - NPM snippet Snippet update required or suggested
Projects
None yet
Development

No branches or pull requests

2 participants