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] AI + MSAL in SPA throws "string value is too long" exception after login. #2167

Open
randompixel opened this issue Sep 26, 2023 · 3 comments

Comments

@randompixel
Copy link

Description/Screenshot
When logging in to a SPA / JS application with MSAL (3.0.2) it passes the "code" back in the address bar as a GET parameter. Since upgrading to 3.x branch of ApplicationInsightsJS this now throws an uncaught exception where AI can't process the request as the URL is too long and it says it is truncating this to 1024 characters.
image

I'm ok with the string being truncated but having the uncaught exception thrown after every login is a bit annoying.

Steps to Reproduce

  • OS/Browser: Any
  • SDK Version [e.g. 22]: v3.0.3
  • How you initialized the SDK:

Expected behavior
Doesn't throw an error or allows us to configure a longer string length.

@MSNev
Copy link
Collaborator

MSNev commented Sep 26, 2023

The error is being thrown because you have enabled the "enableDebug" setting turned on (it's off by default).

With this setting it will cause all calls to throwInternal to be thrown rather than just logged, if this setting to disabled it will just "log" the warning.

This affects not only v3.x but also v2.x because when we did not truncate the string it cause the logged event to be dropped.

I assume, you are requesting that we also provide the ability to selectively define at what level we should throw when the enableDebug is enabled?

@randompixel
Copy link
Author

Hey @MSNev ,

Thanks for that.

  1. Yes please to the length if that's possible
  2. Even if it is triggered by enableDebug, I wouldn't expect this to throw an "uncaught exception" - especially one I'm not sure if I'm meant to try and catch or not? This feels more of a warning kind of thing rather than an Exception?

@MSNev
Copy link
Collaborator

MSNev commented Sep 26, 2023

For the length, that not possible as they are generic messages so they will need to continue calling the function they do now which (depending on the configuration) cause the SDK to emit events with the warning.

On the enableDebug its whole purpose (as currently defined) is to always throw with the value it would normally send as an event, so that it causes the uncaught exception. ie. the enableDebug SHOULD ONLY be used during development and shoudl not be enabled for a production deployment.

I'll tag this issue as an enhancement to provide an additional configuration to enable defining the minimum level to throw @ so if it doesn't throw it will continue to log the event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants