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

Critical ErrorResult #140

Closed
EnochMtzR opened this issue Jun 16, 2023 · 0 comments · Fixed by #141
Closed

Critical ErrorResult #140

EnochMtzR opened this issue Jun 16, 2023 · 0 comments · Fixed by #141

Comments

@EnochMtzR
Copy link
Contributor

EnochMtzR commented Jun 16, 2023

Specific Case:

The Issue:

Currently, there is no ResultStatus that may be mapped to the 500 error code since the Error ResultStatus maps (quite rightly, in my opinion) to a 422 Unprocessable error code.

However, I have the following specific case:

public async Task<ActionResult<Result>> RegisterTaxInfoForCurrentCustomer(ClaimsPrincipal user,
        NewCorporateTaxInfoRequest request)
    {
        try
        {
            var customer = await FindCustomerFromAuthenticatedUser(user);

            if (customer is null)
                return Result.NotFound("Customer not found");

            var taxAddress = GetTaxAddressFromCustomerById(customer, request.TaxAddressId);

            if (taxAddress is null)
                return Result.NotFound("Tax address not found");

            var taxInfo = request.ToCorporateTaxInfo(customer, taxAddress);

            customer.AddTaxInfo(taxInfo);
            await _customerRepository.UpdateAsync(customer);

            return Result.Success();
        }
        catch (UserIdCannotBeRetrieved exception)
        {
            Console.WriteLine(exception);
            return Result.Error("Authentication failed");
        }
        catch (InvalidTaxAddressId exception)
        {
            Console.WriteLine(exception);
            return exception.ToInvalidResult(nameof(request.TaxAddressId));
        }
        catch (InvalidTaxId exception)
        {
            Console.WriteLine(exception);
            return exception.ToInvalidResult(nameof(request.TaxId));
        }
    }

The UserIdCannotBeRetireved exception is thrown when the IDP did not set the sub claim. This, in my opinion, is a critical error and should return an InternalServerError (status 500) since this is not an issue with anything the user has done.

The Proposal:

A new CriticalError ResultStatus that maps to an InternalServerError (500) status code.

I have a pull request with my take on this issue: #141

EnochMtzR added a commit to EnochMtzR/Result that referenced this issue Jun 16, 2023
@EnochMtzR EnochMtzR linked a pull request Jun 18, 2023 that will close this issue
ardalis added a commit that referenced this issue Sep 13, 2023
* Added CriticalError to ResultStatuses (#140)

* Updated tests

---------

Co-authored-by: JosnocSackra <josnocpp@gmail.com>
Co-authored-by: AVANADE-CORP\r.bajor <r.bajor@avanade.com>
Co-authored-by: Steve Smith <steve@kentsmiths.com>
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 a pull request may close this issue.

1 participant