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

[Feature Request] MsalServiceException should allow setting of Headers #1977

Closed
davidmarek opened this issue Aug 5, 2020 · 3 comments
Closed

Comments

@davidmarek
Copy link

Is your feature request related to a problem? Please describe.
I am unable to test my retry logic based on returned headers from AAD because I can't mock MsalServiceException with headers.
Headers and maybe some other properties of MsalServiceException have internal setter and there is no constructor that would set them.

Describe the solution you'd like
MsalServiceException should have a constructor for testing (like AuthenticationResult) that allows setting the headers and all other properties.

Describe alternatives you've considered

  • I tried to mock the exception, but it's not possible to mock field with internal setter.
  • I also tried to create my custom exception class that inherits from MsalServiceException, Headers property can't be overriden and hiding it doesn't work for my case, because the unit under test casts received exceptions to MsalServiceException.
  • I could create my own custom exception type and wrap the code calling MSAL with try catch and wrap the exception. This approach would probably work, it's ugly though and it could cause issues if I try to catch generic MsalException.
@davidmarek
Copy link
Author

I found a possible workaround using reflection:

var ex = new MsalServiceException(MsalError.RequestTimeout, "Timeout");
var res = new HttpResponseMessage();
res.Headers.RetryAfter = delta.HasValue ? new RetryConditionHeaderValue(TimeSpan.FromMinutes(1));
typeof(MsalServiceException).GetProperty("Headers").SetValue(ex, res.Headers, null);

@bgavrilMS bgavrilMS added this to Todo/Committed in MSAL.NET (legacy) via automation Aug 5, 2020
@bgavrilMS bgavrilMS added this to the 4.18.0 milestone Aug 5, 2020
@bgavrilMS
Copy link
Member

Thanks @davidmarek , I'll mark this as a bug because it is blocking testing.

@bgavrilMS bgavrilMS added the P2 label Aug 5, 2020
@bgavrilMS bgavrilMS moved this from Todo/Committed to In progress in MSAL.NET (legacy) Aug 18, 2020
@bgavrilMS bgavrilMS moved this from In progress to Fixed in MSAL.NET (legacy) Aug 19, 2020
@trwalke trwalke modified the milestones: 4.18.0, 4.18.1, 4.19.0 Sep 2, 2020
@trwalke trwalke moved this from Fixed to In progress in MSAL.NET (legacy) Sep 24, 2020
@trwalke trwalke moved this from In progress to Fixed in MSAL.NET (legacy) Sep 24, 2020
@trwalke trwalke added the Fixed label Sep 25, 2020
@trwalke trwalke closed this as completed Sep 25, 2020
@trwalke
Copy link
Member

trwalke commented Sep 25, 2020

Fixed in 4.18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

3 participants