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

DownstreamAPI does not indicate API failures unless trying to deserialize to an output object #2426

Closed
MattCost opened this issue Sep 4, 2023 · 3 comments
Labels
question Further information is requested

Comments

@MattCost
Copy link
Contributor

MattCost commented Sep 4, 2023

Microsoft.Identity.Web Library

Microsoft.Identity.Web

Microsoft.Identity.Web version

2.11.1

Web app

Sign-in users and call web APIs

Web API

Protected web APIs call downstream web APIs

Token cache serialization

Not Applicable

Description

When using any of the convenience methods in DownstreamApi.HttpMethods.cs, if you do not try to deserialize the response into an object, then you will never get the return code from the API.

In the downstream api code, the only place response.EnsureSuccessStatusCode() is called is in the private DeserializeOutput method.

In order to call an api endpoint that only returns a status code one must call CallApiForUser, and manually set the http method, and then manually check for success on the response.

It appears that all the convenience methods in HttpMethods are setup to catch the HttpRequestException the only thing missing is to call EnsureSuccessStatusCode on the response.

Reproduction steps

Use DeleteForUserAsync<TOutput> to call any api that returns 404.
There is no indication that the call failed.

Error message

No response

Id Web logs

No response

Relevant code snippets

public async Task<IActionResult> OnPostDeleteAsync()
        {
            Logger.LogTrace("Entering OnPostDeleteAsync");
            try
            {
                await API.DeleteForUserAsync("API", string.Empty, options =>{
                    options.RelativePath = $"RideEvents/{Id}";
                });
            }
            catch(Exception ex)
            {
                Logger.LogError(ex, "Exception trying to delete RideEvent Id {Id}", Id);
                PreviousPageAction = "RideEvent/Edit/OnPostDelete";
                PreviousPageErrorMessage = $"Error deleting RideEvent. {ex.Message}";
            }
            return RedirectToPage("Index");
        }

Regression

No response

Expected behavior

I expect API.DeleteForUserAsync to throw an HttpResponseException with details of the non-success error code the api returned.

@MattCost MattCost added the question Further information is requested label Sep 4, 2023
@DaleyKD
Copy link

DaleyKD commented Sep 6, 2023

Curious. I'm currently on 2.13.3, and when I use:

return _accountsApi.CallApiForAppAsync<AccountModel>(this.ServiceName, optionsAction, cancellationToken);

It throws an exception.

In fact, I'm looking and looking, trying to find the proper way to handle 404s (my REST API returns a 404 if the specific account isn't found by ID).

Am I missing the point of your post? (Any ideas on how to handle 404s? 😄 )

@MattCost
Copy link
Contributor Author

MattCost commented Sep 6, 2023

CallApiForAppAsync / CallApiForUserAsync methods DO call response.EnsureSuccessStatusCode() and throw an exception like you are seeing.

GetForUser - also calls response.EnsureSuccessStatusCode()
The non-get calls that don't try deserialize into an object (DeleteForUser / PostForUser / etc) - don't call response.EnsureSuccessStatusCode(). So if you don't return an object. so you get a silent failure.

What you want to do, is catch the ex, and handle the 404

try
{
    return _accountsApi.CallApiForAppAsync<AccountModel>(this.ServiceName, optionsAction, cancellationToken);
}
catch(HttpResponseException ex) when (ex.Message.Contains("404"))
{
    return null; 
    // or throw new Exception("Account ${id} not found");
    //or whatever you want to do
}

@MattCost MattCost closed this as completed Sep 6, 2023
@jennyf19
Copy link
Collaborator

jennyf19 commented Sep 8, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants