Assertion triggered on AWSProfileConfigLoader without Aws::InitAPI

[cosunae]

Confirm by changing [ ] to [x] below to ensure that it's a bug:

• [ x ] I've gone though Developer Guide and API reference
• [ x ] I've searched for previous similar issues and didn't find any solution

Describe the bug
I was following the tutorial here, https://docs.aws.amazon.com/sdk-for-cpp/v1/developer-guide/examples-cloudwatch-publish-custom-metrics.html.
Without realizing it was not a complete example.
Without the line
Aws::InitAPI(options);
the constructor of CloudWatchClient seg faults in release mode. When I compiled aws-sdk-cpp in debug mode I obtained the stack trace I post below and finally realized that there was some initialization missing.

The assertion triggered is this one:

aws-sdk-cpp/aws-cpp-sdk-core/source/config/AWSProfileConfigLoader.cpp

Line 512 in 87a8456

assert(s_configManager);

I would consider this as a bug, since I would expect the user gets (for a misconfiguration or ill-formed user program like this, since it does not follow the initialization as described in the docs) either a runtime exception or an error, not an assertion, or worse, in release a seg fault.
Btw, I am aware that aws-sdk-cpp does not use exceptions.

SDK version number
87a8456

Platform/OS/Hardware/Device
ubuntu 18.04, g++-8

To Reproduce (observed behavior)
int main() {
Aws::CloudWatch::CloudWatchClient cw;
}

Expected behavior
I would not expect an assertion, but rather a runtime exception or error code return (which is not possible in a constructor unless the pattern is moved into a maker function).

Logs/output
error stack trace, when compiling aws-sdk-cpp in debug mode
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff67e18b1 in __GI_abort () at abort.c:79
#2 0x00007ffff67d142a in __assert_fail_base (fmt=0x7ffff6958a38 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x7ffff7616760 "s_configManager",
file=file@entry=0x7ffff7616710 "/software/aws-sdk-cpp/aws-cpp-sdk-core/source/config/AWSProfileConfigLoader.cpp", line=line@entry=512,
function=function@entry=0x7ffff7619780 <Aws::Config::GetCachedConfigValue(std::__cxx11::basic_string<char, std::char_traits, Aws::Allocator > const&)::PRETTY_FUNCTION> "Aws::String Aws::Config::GetCachedConfigValue(const String&)") at assert.c:92
#3 0x00007ffff67d14a2 in __GI___assert_fail (assertion=0x7ffff7616760 "s_configManager", file=0x7ffff7616710 "/software/aws-sdk-cpp/aws-cpp-sdk-core/source/config/AWSProfileConfigLoader.cpp", line=512,
function=0x7ffff7619780 <Aws::Config::GetCachedConfigValue(std::__cxx11::basic_string<char, std::char_traits, Aws::Allocator > const&)::PRETTY_FUNCTION> "Aws::String Aws::Config::GetCachedConfigValue(const String&)") at assert.c:101
#4 0x00007ffff75a7e46 in Aws::Config::GetCachedConfigValue (key="max_attempts") at /software/aws-sdk-cpp/aws-cpp-sdk-core/source/config/AWSProfileConfigLoader.cpp:512
#5 0x00007ffff75a3380 in Aws::Client::ClientConfiguration::ClientConfiguration (this=0x7fffffffce80) at /software/aws-sdk-cpp/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp:69

[KaibaLopez]

Hi @cosunae
I don't really agree with this being a bug on the sdk, and I get this isn't the problem on this issue but just to clarify, because I think the page is unnecessarily confusing.
When you say "Without realizing it was not a complete example." does this mean that eventually you found the full code link under the code snippet? if not, you can find the full sample here.

And back to the asserts, they are there to prevent "impassable" states, such as in this case of "not initializing the sdk" now granted that we could maybe have better error messages when we hit those, but this is how the sdk was designed to function, so...yea using asserts is not a bug.

Let me know what you think, and also if I misunderstood something.

[cosunae]

Hi @KaibaLopez

thanks for the reply. Let me clarify the confusion. After I compiled the sdk in debug mode and found the assertion, yes, I found the full code and yes, and my problem was solved. So I dont need help to get it to run. I only posted the issue in order to contribute to improve this, since I guess others might fall into the same issue.
If you believe this is not a bug, then please feel free to close the issue.

In my opinion, any user program that misuses a library and get a seg fault without returning an error or exception is a bug of the library. The user is not supposed to compile the sdk in debug mode in order to find an assertion and figure out why this assertion was triggered. I agree that assertions should be used to protect from wrong states of the program, but they should be meant mainly for developers, to detect those states generated by internal errors of the library.
So of course using assertions is not the bug, but rather a user triggering an assertion without any error or exception returned.
And to me it does not look like the sdk was designed without error handling, or at least I have found some examples doing what I would expect

auto outcome = cw.ListMetrics(request);
    if (!outcome.IsSuccess())
    {
        std::cout << "Failed to list CloudWatch metrics:" <<
            outcome.GetError().GetMessage() << std::endl;
        break;
    }

[KaibaLopez]

Yea I guessed you got it but wanted to make sure just in case. And while we may not be agreeing I for one really appreciate you taking the time to write out your opinion, gives me a reason to go through and review things.

That being said, I think the part I'm not agreeing here with is :

to detect those states generated by internal errors of the library.

This specific error happens because we allow the user to initialize and shutdown whenever it works best, but we also need to make sure the states are correct when ran. We could have regular checks but those would slow down the code significantly, and so asserts come in handy.

The user is not supposed to compile the sdk in debug mode in order to find an assertion and figure out why this assertion was triggered.
True, we assume the user will read the usage guidance and documentation though, it is arguably a big assumption, specially considering the documentation can be confusing, but yea if used properly it should not get to that.

I'll be closing the issue, but I want to say, thanks again for the feedback, and if you have a specific idea on how this could be better I'd be happy to review a PR.