DtoUtils.CreateErrorResponse Bug

I have a service that uses a MapExceptionToStatusCode like so:

            appHost.SetConfig(new HostConfig
            {
                MapExceptionToStatusCode = new Dictionary<Type, int>
				{
					{ typeof(ValidationError), 400 },
					{ typeof(ArgumentException), 400 },
					{ typeof(ArgumentNullException), 400 },
					{ typeof(ArgumentOutOfRangeException), 400 },
					{ typeof(RuleViolationException), 400 },
					{ typeof(AuthenticationException), 401 },
					{ typeof(UnauthorizedAccessException), 401 },
					{ typeof(ForbiddenException), 403 },
					{ typeof(ResourceNotFoundException), 404 },
					{ typeof(RoleViolationException), 404 },
					{ typeof(MethodNotAllowedException), 405 },
					{ typeof(ResourceConflictException), 409 },
				},
            });

The service also defines a ServiceExceptionHandler like so:

            appHost.ServiceExceptionHandlers.Add((req, request, ex) =>
				{
					if (ex.ToStatusCode() >=500)
					{
						//Logging 'unexpected' exception here

                                                // Creating a wrapped, safe, sanitized 500 exception
						var unexpected = new Exception(string.Empty, new UnexpectedException(ex));
						return DtoUtils.CreateErrorResponse(request, unexpected);
					}

					return null;
				};
			);

The semantics here are that:

  • If the exception is one of the ‘expected’ types my service knows about throwing, then I want to return a ResponseStatus with the status code listed in MapExceptionToStatusCode.
  • else, I want to return a 500 ResponseStatus and sanitize the message and content of the exception.

As per the docs Error Handling, it recommends that I should return null from my ServiceExceptionHandler if I want SS to do the right thing with my ‘expected’ exceptions and return the right ResponseStatus (as per MapExceptionToStatusCode).
Which I do if the exception is one of the expected exception type listed in MapExceptionToStatusCode list, else I call DtoUtils.CreateErrorResponse() directly with my ‘unexpected’ exception, which should represent all other exceptions.

Now, I understand from the code that when I return null from this handler, SS will eventually call DtoUtils.CreateErrorResponse() to create the correct response on the wire.
And it is then that I get see the bug.

To demonstrate the bug, if my service throws a UnauthorizedAccessException, I am expecting to get a 401 ResponseStatus.
But in fact I get a 500 ResponseStatus, and with StackTrace on I get a NullReferenceException like this:

{"ResponseStatus":{"ErrorCode":"NullReferenceException","Message":"Object reference not set to an instance of an object.","StackTrace":"   at ServiceStack.DtoUtils.CreateErrorResponse(Object request, Exception ex)\r\n   at ServiceStack.Host.ServiceRunner`1.HandleException(IRequest request, TRequest requestDto, Exception ex)\r\n   at ServiceStack.Host.ServiceRunner`1.Execute(IRequest request, Object instance, TRequest requestDto)\r\n   at ServiceStack.Host.ServiceExec`1.Execute(IRequest request, Object instance, Object requestDto, String requestName)\r\n   at ServiceStack.Host.ServiceRequestExec`2.Execute(IRequest requestContext, Object instance, Object request)\r\n   at ServiceStack.Host.ServiceController.ManagedServiceExec(ServiceExecFn serviceExec, IService service, IRequest request, Object requestDto)\r\n   at ServiceStack.Host.ServiceController.<>c__DisplayClass36_0.<RegisterServiceExecutor>b__0(IRequest req, Object dto)\r\n   at ServiceStack.Host.ServiceController.Execute(Object requestDto, IRequest req)\r\n   at ServiceStack.HostContext.ExecuteService(Object request, IRequest httpReq)\r\n   at ServiceStack.Host.RestHandler.GetResponse(IRequest request, Object requestDto)\r\n   at ServiceStack.Host.RestHandler.ProcessRequestAsync(IRequest httpReq, IResponse httpRes, String operationName)"}}

Here is why:

  1. When DtoUtils.CreateErrorResponse is called with a UnauthorizedAccessException, line 1 converts the exception from a UnauthorizedAccessException to a WebServiceException because in this case my UnauthorizedAccessException has an InnerException of type WebServiceException (it is chained call to another service from a ServiceClient), and ResolveResponseException returns the InnerException property, which is a WebServiceException.
  2. Then, line 2 calls DtoUtils.ToResponseStatus which decides that the exception type is IResponseStatusConvertible and return the IResponseStatusConvertible.ResponseStatus, which in this case is null
  3. Then line 3, if HostContext.DebugMode sets the exception StackTrace property on the null instance, which then ultimately throws the NullReferenceException down the track.

and hence the service spits out a 500 response, not the expected 401 response.

if HostContext.DebugMode == false, the service does spit out the 401 status code, but the body of the response is not incorrect, and not a valid ResponseStatus.

Now, I am not sure of the fix exactly, because I am not sure of the correct intentions/assumptions of the first few lines of DtoUtils.CreateErrorResponse. But in this case, ToResponseStatus() is returning null on the converted WebServiceException.

Can someone suggest a fix please?

Could the fix in DtoUtils.CreateErrorResponse be as simple as:

on line 140: check if the responseStatus is null, and if so, then re-use the original exception passed into the method, and call ToResponseStatus() on that instead?

i.e:

        public static object CreateErrorResponse(object request, Exception ex)
        {
            var originalEx = ex;
            ex = HostContext.AppHost?.ResolveResponseException(ex) ?? ex;
            var responseStatus = ex.ToResponseStatus();

///FIX
            if (responseStatus == null)
            {
                ex = originalEx;
                responseStatus = ex.ToResponseStatus();
            }
///FIX

            if (HostContext.DebugMode)
            {
                // View stack trace in tests and on the client
                responseStatus.StackTrace = GetRequestErrorBody(request) + "\n" + ex;
            }

            HostContext.AppHost?.OnLogError(typeof(DtoUtils), "ServiceBase<TRequest>::Service Exception", ex);

            var errorResponse = CreateErrorResponse(request, ex, responseStatus);

            HostContext.AppHost?.OnExceptionTypeFilter(ex, responseStatus);

            return errorResponse;
        }

I think this fix can be applied, but it needs to be reviewed by @mythz. By the way you can disable conversion exception to inner exception and avoid NRE if set HostConfig.ReturnsInnerException to false in the Configure method.

        SetConfig(new HostConfig { ReturnsInnerException = false, DebugMode = true });

Yeah I think that’s a good fallback which I’ve added in this commit. Sergey’s answer of Config.ReturnsInnerException = false will prevent it from using the Inner Exception.

Great thanks Demis,

Can I ask, what is the true intention of Config.ReturnsInnerException?
Surely not just for use by CreateErrorResponse is it?
I mean, can you give some more details on what scenarios are you trying to support with it?

Was asked years ago to return the InnerException if it exists as it would often be more informative on what the cause of the Error was. This default can be overridden with Config.ReturnsInnerException = false.