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

It is always "Retrieving configuration..." when adding one custom environment if using 'https://maneg.com' as the ARM endpoint #7899

Open
v-kellyluo opened this issue Apr 25, 2024 · 5 comments
Assignees
Labels
🪲 bug Issue is not intended behavior 🧪 testing Found through regular testing
Milestone

Comments

@v-kellyluo
Copy link

v-kellyluo commented Apr 25, 2024

Storage Explorer Version: 1.34.0-dev (98)
Build Number: 20240425.1
Branch: main
Platform/OS: Windows 10/Linux Ubuntu 22.04/MacOS Sonoma 14.4.1(Apple M1 Pro)
Architecture: x64/x64/arm64
How Found: Ad-hoc testing
Regression From: Previous release (1.30.2)

Steps to Reproduce

  1. Launch Storage Explorer
  2. Open the connect dialog -> Select 'Subscription -> Custom Environment:' -> Click 'Manage custom environments...'.
  3. Click 'Add' -> Type a valid environment name.
  4. Type 'https://maneg.com' into ARM endpoint field -> Click 'Add'.
  5. Check whether an error message appears.

Expected Experience

An error message appears.
image

Actual Experience

It is always in a verified state.
image

@v-kellyluo v-kellyluo added 🧪 testing Found through regular testing 🪲 regression Issue was working in a previous version labels Apr 25, 2024
@craxal craxal self-assigned this Apr 25, 2024
@craxal craxal added this to the 1.34.0 milestone Apr 25, 2024
@craxal
Copy link
Contributor

craxal commented Apr 25, 2024

@v-kellyluo This does not reproduce for me at all using the same build or on our main branch. Clicking Add does require network communication. Perhaps there was a network issue? Does this still reproduce for you?

Image

@v-kellyluo
Copy link
Author

Hi @craxal, we waited more than 10 minutes, this issue still reproduces on our side, and it doesn't reproduce in 1.30.2 on the same machine.

@craxal
Copy link
Contributor

craxal commented Apr 26, 2024

@v-kellyluo I still don't reproduce. Can you provide app logs? Test machine?

@craxal craxal added the ❔ more info More information is needed from issue author to proceed label Apr 26, 2024
@craxal craxal changed the title It is always in a verified state when adding one custom environment if using 'https://maneg.com' as the ARM endpoint It is always "Retrieving configuration..." when adding one custom environment if using 'https://maneg.com' as the ARM endpoint Apr 26, 2024
@v-kellyluo
Copy link
Author

@craxal , here is the logs, and I will share the test machine in Teams.
2024-04-27_172306.zip

@craxal
Copy link
Contributor

craxal commented Apr 29, 2024

There's a "TypeError: Converting circular structure to JSON" error getting thrown that isn't being caught.

Here's what I think is happening:

  1. The management endpoint is passed to the function for adding custom environments.
  2. The management is passed to the identity library which calls into a custom HTTP client to send a request for the management endpoint configuration.
  3. Because the endpoint does not exist, the HTTP request throws an error.
  4. The custom HTTP client handles errors by catching them, logging them, then rethrowing the error. The error is passed to our ExceptionSerializer.serialize() function. This, I think, is what is throwing, because it's trying to serialize a property (issuerCertificate) that isn't JSON-serializable (circular or self-referencing).

This is...a problem. I don't think we ever anticipated any error or error-like objects to contain circular references. We'll need to handle this somehow at some point.

To clarify, though, I do not believe this is a regression, as the circular references issue has always been there; it's just surfacing now, for some reason. Also, considering this only happens when the management endpoint doesn't exist, and we don't accept them until we've gotten positive confirmation that it does, I don't think a fix for this is urgent.

@craxal craxal added 🪲 bug Issue is not intended behavior and removed 🪲 regression Issue was working in a previous version ❔ more info More information is needed from issue author to proceed labels Apr 29, 2024
@craxal craxal modified the milestones: 1.34.0, 1.35.0 Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Issue is not intended behavior 🧪 testing Found through regular testing
Projects
None yet
Development

No branches or pull requests

2 participants