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

Install report hook in StoreAPI #300

Merged
merged 3 commits into from
Sep 26, 2023
Merged

Conversation

EduardGomezEscandell
Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell commented Sep 26, 2023

Mostly copy-pasted from ubuntu/WSL#404

Note that in WSL, we simply exited with exit(EXIT_FAILURE);. However, this causes the go test framework to crash. Instead, I decided to use a throw statement so that we get some more info out of the crash (mainly the API call that triggered it).

Example error

Here is an example test failure, after adding these two nonsense lines to GenerateUserJWT:

std::vector<int> hello;
hello[3];

Test logs:

time=2023-09-26T09:46:31.992+02:00 level=INFO msg="Building store api DLL"
time=2023-09-26T09:46:38.662+02:00 level=INFO msg="Built store api DLL"
storeapi: GetSubscriptionExpirationDate: [ERROR]: Query found no products. id=ABCDEFG
C:\Users\edu19\Work\ubuntu-pro-for-windows\storeapi\base\StoreService.hpp:31 class StoreApi::impl::StoreContext::Product __cdecl StoreApi::StoreService<class StoreApi::impl::StoreContext>::GetSubscriptionProduct(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >)
[ASSERT] C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\vector(1886) : Assertion failed: vector subscript out of range


storeapi: GenerateUserJWT: [ASSERT] C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\vector(1886) : Assertion failed: vector subscript out of range


--- FAIL: TestGenerateUserJWT (0.00s)
    --- FAIL: TestGenerateUserJWT/Error_because_there_is_no_subscription (0.01s)
        store_test.go:70:
                Error Trace:    C:/Users/edu19/Work/ubuntu-pro-for-windows/storeapi/go-wrapper/microsoftstore/store_test.go:70
                Error:          Target error should be in err chain:
                                expected: "empty user JWT was generated"
                                in chain: "GenerateUserJWT: GenerateUserJWT: storeApi returned error code -1: unexpected error"
                                        "GenerateUserJWT: storeApi returned error code -1: unexpected error"
                                        "storeApi returned error code -1: unexpected error"
                                        "unexpected error"
                Test:           TestGenerateUserJWT/Error_because_there_is_no_subscription
                Messages:       SubscriptionToken returned an unexpected error type
FAIL
exit status 1
FAIL    github.com/canonical/ubuntu-pro-for-windows/storeapi/go-wrapper/microsoftstore  8.778s

UDENG-1390

}
}();

std::cerr << type << ' ' << message << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rely on the fact that we catch all exceptions at the ABI boundary (and print their messages) to avoid this line causing double-printing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

This also considerably simplifies the code. You may be concerned that I
no longer print the reportType. That is because the message already
contains it.
Copy link
Contributor

@CarlosNihelton CarlosNihelton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏽

@EduardGomezEscandell EduardGomezEscandell merged commit 73355ea into main Sep 26, 2023
30 checks passed
@EduardGomezEscandell EduardGomezEscandell deleted the mvsc-report-hook branch September 26, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants