[azure-identity][azure-messaging-eventhubs] Impossible to catch exception resulting in SIGABRT signal

[chusitoo]

Describe the bug
Some exceptions appear to be thrown in a different thread and, therefore, do not bubble up through the user-facing API.

This seems to be affecting most (or all) classes extending TokenCredential in Azure Identity, although other parts of the SDK might exhibit this behavior as well.

Exception or Stack Trace

terminate called after throwing an instance of 'Azure::Core::Credentials::AuthenticationException'
  what():  GetToken(): error response: 400 Bad Request
Aborted

To Reproduce
Create a credential object with some bogus or invalid input. It could be a simple typo or maybe even an expired credential.

The example below demonstrates this by supplying an invalid tenant id, client id and/or client secret to a ClientSecretCredential, although I was also able to reproduce it with a WorkloadIdentityCredential and an invalid federated token file path or altering the contents of that file. I do realize that it is not meant to be modified by hand, this just is another way of reproducing the crash.

Code Snippet
Modified version of produce_events_aad.cpp that, instead of instantiating a DefaultAzureCredential, creates a ClientSecretCredential with some invalid credentials.

diff --git a/sdk/eventhubs/azure-messaging-eventhubs/samples/produce-events/produce_events_aad.cpp b/sdk/eventhubs/azure-messaging-eventhubs/samples/produce-events/produce_events_aad.cpp
index 2d2bdd2c..b8d64d4d 100644
--- a/sdk/eventhubs/azure-messaging-eventhubs/samples/produce-events/produce_events_aad.cpp
+++ b/sdk/eventhubs/azure-messaging-eventhubs/samples/produce-events/produce_events_aad.cpp
@@ -31,13 +31,19 @@ int main()
     return 1;
   }

-  auto credential = std::make_shared<Azure::Identity::DefaultAzureCredential>();
+  std::shared_ptr<Azure::Messaging::EventHubs::ProducerClient> producerClient;

-  Azure::Messaging::EventHubs::ProducerClient producerClient(
-      eventhubsHost, eventhubName, credential);
+  try {
+    const auto credential = std::make_shared<Azure::Identity::ClientSecretCredential>(
+                                "foo", "bar", "baz");
+    producerClient = std::make_shared<Azure::Messaging::EventHubs::ProducerClient>(
+                        eventhubsHost, eventhubName, credential);
+  } catch (const std::exception& e) {
+      std::cout << "[ Exception ] " << e.what() << std::endl;
+  }

   Azure::Messaging::EventHubs::Models::EventHubProperties eventhubProperties
-      = producerClient.GetEventHubProperties();
+      = producerClient->GetEventHubProperties();

   // By default, the producer will round-robin amongst all available partitions. You can use the
   // same producer instance to send to a specific partition.
@@ -47,7 +53,7 @@ int main()
   // configure this batch processor to send to that partition.
   Azure::Messaging::EventHubs::EventDataBatchOptions batchOptions;
   batchOptions.PartitionId = eventhubProperties.PartitionIds[0];
-  Azure::Messaging::EventHubs::EventDataBatch batch{producerClient.CreateBatch(batchOptions)};
+  Azure::Messaging::EventHubs::EventDataBatch batch{producerClient->CreateBatch(batchOptions)};

   // Send an event with a simple binary body.
   {
@@ -73,5 +79,5 @@ int main()
     batch.TryAddMessage(event);
   }

-  producerClient.Send(batch);
+  producerClient->Send(batch);
 }

I am using the sample source file from the latest main:

$ git rev-parse HEAD
b1796ab919134f12bd2a2bdb25bfa68336f35b74

Expected behavior
The thrown exception is caught and, based on above, something similar to this is printed on the console without terminating the process:

[ Exception ] GetToken(): error response: 400 Bad Request

Setup (please complete the following information):

• OS: CentOS7 / Mariner 2.0
• IDE : vi / VS Code
• Version of the Library used

Package: azure-core-cpp
Version: 1.11.0
--
Package: azure-identity-cpp
Version: 1.6.0
--
Package: azure-macro-utils-c
Version: 2022-01-21
--
Package: azure-c-shared-utility
Version: 2023-08-07
--
Package: azure-core-amqp-cpp
Version: 1.0.0-beta.6
--
Package: azure-messaging-eventhubs-cpp
Version: 1.0.0-beta.5

Additional context
Currently using GCC 11.2.0 but was previously seen on GCC 7.5 as well.

g++ produce_events_aad.cpp \
        -std=c++17 \
        -pthread \
        -lazure-core \
        -lazure-identity \
        -lazure-core-amqp \
        -lazure-messaging-eventhubs \
        -o produce_events_aad

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

• Bug Description Added
• Repro Steps Added
• Setup information Added

[antkmsft]

Hi Alex, please help me to understand if I am missing something.

A few lines in your code look like this:

try
{
  const auto credential
      = std::make_shared<Azure::Identity::ClientSecretCredential>("foo", "bar", "baz");
  producerClient = std::make_shared<Azure::Messaging::EventHubs::ProducerClient>(
      eventhubsHost, eventhubName, credential);
}
catch (const std::exception& e)
{
  std::cout << "[ Exception ] " << e.what() << std::endl;
}

And you are expecting that this line - producerClient = std::make_shared<Azure::Messaging::EventHubs::ProducerClient>(eventhubsHost, eventhubName, credential); - should throw the AuthenticationException.

While in fact, constructing the ProducerClient client will not throw. It is the first line that results in making a network request, will throw that exception, i.e. the following line: Azure::Messaging::EventHubs::Models::EventHubProperties eventhubProperties = producerClient->GetEventHubProperties().

And if you move that line inside your try/catch block as below, it will catch the exception:

Azure::Messaging::EventHubs::Models::EventHubProperties eventhubProperties;
try
{
  const auto credential
      = std::make_shared<Azure::Identity::ClientSecretCredential>("foo", "bar", "baz");
  producerClient = std::make_shared<Azure::Messaging::EventHubs::ProducerClient>(
      eventhubsHost, eventhubName, credential);

  eventhubProperties = producerClient->GetEventHubProperties();
}
catch (const std::exception& e)
{
  std::cout << "[ Exception ] " << e.what() << std::endl;
}

Does this comment makes it clearer, or am I missing it?

[github-actions]

Hi @chusitoo. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

[LarryOsterman]

Alex, We're still trying to understand the scenario that your bug report is describing.

The sample failure included appears to be mitigated by putting a try/catch around the core logic, which isn't "impossible to catch".

I'd like to understand more about how the "impossible to catch" exception is generated.

[chusitoo]

Hi Anton, Larry,

Thanks for the reply and the clarification. I stand corrected here, as I was trying to transpose the code from my project back into the example produce_events_aad.cpp

And, indeed, I was referring to the method GetEventHubProperties() from ProducerClient.

I opted to keep my original message unchanged and provide the updated diff below to keep the conversation coherent:

diff --git a/sdk/eventhubs/azure-messaging-eventhubs/samples/produce-events/produce_events_aad.cpp b/sdk/eventhubs/azure-messaging-eventhubs/samples/produce-events/produce_events_aad.cpp
index 2d2bdd2c..18f54b8a 100644
--- a/sdk/eventhubs/azure-messaging-eventhubs/samples/produce-events/produce_events_aad.cpp
+++ b/sdk/eventhubs/azure-messaging-eventhubs/samples/produce-events/produce_events_aad.cpp
@@ -31,14 +31,11 @@ int main()
     return 1;
   }

-  auto credential = std::make_shared<Azure::Identity::DefaultAzureCredential>();
-
+  auto credential = std::make_shared<Azure::Identity::ClientSecretCredential>(
+                                "foo", "bar", "baz");
   Azure::Messaging::EventHubs::ProducerClient producerClient(
       eventhubsHost, eventhubName, credential);

-  Azure::Messaging::EventHubs::Models::EventHubProperties eventhubProperties
-      = producerClient.GetEventHubProperties();
-
   // By default, the producer will round-robin amongst all available partitions. You can use the
   // same producer instance to send to a specific partition.
   // To do so, specify the partition ID in the options when creating the batch.
@@ -46,7 +43,18 @@ int main()
   // The event consumer sample reads from the 0th partition ID in the eventhub properties, so
   // configure this batch processor to send to that partition.
   Azure::Messaging::EventHubs::EventDataBatchOptions batchOptions;
-  batchOptions.PartitionId = eventhubProperties.PartitionIds[0];
+  std::cout << "Before try" << std::endl;
+
+  try {
+    Azure::Messaging::EventHubs::Models::EventHubProperties eventhubProperties
+      = producerClient.GetEventHubProperties();
+    std::cout << "Inside try" << std::endl;
+    batchOptions.PartitionId = eventhubProperties.PartitionIds[0];
+  } catch (...) {
+      std::cout << "Caught exception from call to GetEventHubProperties()" << std::endl;
+  }
+
+  std::cout << "After try" << std::endl;
   Azure::Messaging::EventHubs::EventDataBatch batch{producerClient.CreateBatch(batchOptions)};

   // Send an event with a simple binary body.

What I observe running the above is:

# ./produce_events_aad
Before try
Aborted

The rest is essentially the same as before; if I remove the try altogether, the process terminates with:

terminate called after throwing an instance of 'Azure::Core::Credentials::AuthenticationException'
  what():  GetToken(): error response: 400 Bad Request
Aborted

I hope this makes the intent somewhat clearer. Thanks in advance!

[LarryOsterman]

Ah - I think I figured the problem out.

After the authentication exception is thrown, we attempt to clean up the state of the various AMQP objects, one of which is asserting because an invariant condition is not met (the object is being destroyed while it is still open, but the contract for this object requires that it be explicitly closed once opened). To indicate this programming error, the function calls the C runtime library function abort() which eventually calls terminate().

So technically there isn't an impossible to catch exception being thrown. Instead, the application is being terminated by the runtime after an exception is thrown.

I'll get to work on a fix for the problem.

[chusitoo]

That's good to hear! I appreciate you taking the time to look into this.

[github-actions]

Hi @chusitoo, since you haven’t asked that we /unresolve the issue, we’ll close this out. If you believe further discussion is needed, please add a comment /unresolve to reopen the issue.

[chusitoo]

Hi @chusitoo, since you haven’t asked that we /unresolve the issue, we’ll close this out. If you believe further discussion is needed, please add a comment /unresolve to reopen the issue.

I believe this may not be fixed yet so I would like to /unresolve for visibility and ease of tracking

[LarryOsterman]

Thank you for reopening this. FWIW, I'm testing the fix for this as a part of my most recent AMQP/EventHubs changes.