microsoft / AzureStorageExplorer

Easily manage the contents of your storage account with Azure Storage Explorer. Upload, download, and manage blobs, files, queues, tables, and Cosmos DB entities. Gain easy access to manage your virtual machine disks. Work with either Azure Resource Manager or classic storage accounts, plus manage and configure cross-origin resource sharing (CORS) rules.
Creative Commons Attribution 4.0 International
365 stars 85 forks source link

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

Open v-kellyluo opened 2 months ago

v-kellyluo commented 2 months ago

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

craxal commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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

v-kellyluo commented 2 months ago

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

craxal commented 2 months ago

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.