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

Every requests frombody parameter became empty #22

Closed
attilaszabo2 opened this issue Jan 10, 2020 · 11 comments
Closed

Every requests frombody parameter became empty #22

attilaszabo2 opened this issue Jan 10, 2020 · 11 comments
Labels

Comments

@attilaszabo2
Copy link

@attilaszabo2 attilaszabo2 commented Jan 10, 2020

Hi,

I found an edge case when your middleware causing a weird problem.
In my .net core 3.1 application I tried to create HttpClient from WebApplicationFactory because I wanted to write an integration test for a POST request.
The problem is that this request is containing a (FromBody) parameter, which could not be found after I was registered your middleware btw I'm using a second middleware. Actually I don't really know what is the root cause of the issue but I found the solution.
Can you please change the following code in the AutoWrapperMembers.FormatRequest method?
from

var buffer = new byte[Convert.ToInt32(request.ContentLength)];
await request.Body.ReadAsync(buffer, 0, buffer.Length);
var bodyAsText = Encoding.UTF8.GetString(buffer);
request.Body.Seek(0, SeekOrigin.Begin);

to

request.Body.Seek(0, SeekOrigin.Begin);
request.Body.CopyToAsync(stream);
bodyAsText = Encoding.UTF8.GetString(stream.ToArray());
request.Body.Seek(0, SeekOrigin.Begin);

Because it looks like that the request.Body.ReadAsync closes the request body stream anyway, even if you executing the request.Body.Seek().

Best regard,
Attila

@proudmonkey

This comment has been minimized.

Copy link
Owner

@proudmonkey proudmonkey commented Jan 10, 2020

Hi @attilaszabo2 - Thank you for the feedback!

I may have to look into it and do some testing to verify it. I haven't tried using WebApplicationFactory for this and I'm not certain that the line that you highlighted causes the body parameter to be empty.

Also, what is the second middleware that you are using?

@attilaszabo2

This comment has been minimized.

Copy link
Author

@attilaszabo2 attilaszabo2 commented Jan 13, 2020

The other middleware is a custom one, which does nothing, it is just a secondary exception handler, which handles the custom backend exceptions and cast them to ApiException. But basically it is just calling the Next(httpContext).

@proudmonkey

This comment has been minimized.

Copy link
Owner

@proudmonkey proudmonkey commented Jan 13, 2020

What happens if your comment out the other middleware?

@attilaszabo2

This comment has been minimized.

Copy link
Author

@attilaszabo2 attilaszabo2 commented Jan 14, 2020

Unfortunately nothing, I still get the same response:
{"type":"https://tools.ietf.org/html/rfc7231#section-6.5.1","title":"One or more validation errors occurred.","status":400,"traceId":"|dcce2500-48a915dd068da960.","errors":{"$":["The input does not contain any JSON tokens. Expected the input to start with a valid JSON token, when isFinalBlock is true. Path: $ | LineNumber: 0 | BytePositionInLine: 0."],"login":["The login field is required."]}}

empty_response

@proudmonkey

This comment has been minimized.

Copy link
Owner

@proudmonkey proudmonkey commented Jan 14, 2020

@attilaszabo2 Thank you. I'll look deeper into this.

@attilaszabo2

This comment has been minimized.

Copy link
Author

@attilaszabo2 attilaszabo2 commented Jan 15, 2020

@proudmonkey I actually created a new feature branch, where I added a simple webapi app and wrote the integration test, so now you can check it, reproduce the error or even debug.
See here: https://github.com/attilaszabo2/AutoWrapper/tree/features/integration_tests

@proudmonkey

This comment has been minimized.

Copy link
Owner

@proudmonkey proudmonkey commented Feb 3, 2020

@attilaszabo2 I'm able to replicate it. I'm refactoring the code and will roll-out a new version release soon. I'll keep you posted. Thank you for you feedback! :)

@arhen

This comment has been minimized.

Copy link
Contributor

@arhen arhen commented Feb 13, 2020

Sorry, I think this is not relate to this issues, but
Hi @proudmonkey , can I show ur example code of ur custom exception that u already cast to ApiException. Before I use AutoWrapper, I've my own middleware of custom exception. Then when I notice that there is no option to disable AutoWrapper exception and use my own instead, I started to think to modify my own then cast to ApiException.

@proudmonkey

This comment has been minimized.

Copy link
Owner

@proudmonkey proudmonkey commented Feb 13, 2020

@arhen Yes. You can define your own Error object and pass it to the ApiException() method.

@arhen

This comment has been minimized.

Copy link
Contributor

@arhen arhen commented Feb 13, 2020

@arhen Yes. You can define your own Error object and pass it to the ApiException() method.

This is interesting.
let's tlak about it a little bit.
So I have my own custom exception middleware handler that I previously made it as Global handling error before use AutoWrapper like this.

public class ExceptionMiddleware
{
    private readonly RequestDelegate _next;
    private readonly ILoggerManager _logger;

    public ExceptionMiddleware(RequestDelegate next, ILoggerManager logger)
    {
        _logger = logger;
        _next = next;
    }

    public async Task InvokeAsync(HttpContext httpContext)
    {
        try
        {
            await _next(httpContext);
        }
        catch (Exception ex)
        {
            _logger.LogError($"Something went wrong: {ex}");
            //await HandleExceptionAsync(httpContext, ex); //my previous method to handle exception
           await HandleApiExceptionAsycn(httpContext ex); //This is my method that I want to the exception wrapping out by ApiException from AutoWrapper
        }
    }

    private Task HandleExceptionAsync(HttpContext context, Exception exception)
    {
        context.Response.ContentType = "application/json";
        context.Response.StatusCode = (int)HttpStatusCode.InternalServerError;

        return context.Response.WriteAsync(new ErrorDetails()
        {
            StatusCode = context.Response.StatusCode,
            Message = "Internal Server Error."
        }.ToString());
    }

   private task HandleApiExceptionAsync(HttpContext context, Exception exception)
    {
        //IMPLEMENT ApiException handling from AutoWrapper
    }

    
    

}

then register app.UseMiddleware<ExceptionMiddleware>(); to app pipeline on startup.cs.

As u can see above, I have no idea to implement my own custom and pass it to ApiException then use this middleware as ExceptionHandler on pipelane.

I'm using ErrorDetails model to define my custom error messsage. I've tried to cast my own ErrorDetails model to ApiException inside that HandleApiExceptionAsync method, but still since I've use app.UseApiResponseAndExceptionWrapper() on startup, this middleware never work out and always use Exception from AutoWrapper instead.

@proudmonkey

This comment has been minimized.

Copy link
Owner

@proudmonkey proudmonkey commented Feb 26, 2020

@attilaszabo2 @arhen

Just released a new version that comes with a fix for this issue. You can find it here: AutoWrapper Now Supports Problem Details For Your ASP.NET Core APIs

Thank you so much for all your suggestions on improving this middleware! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.