Middleware not working for 400 BadRequest

[SoftwareDreamer]

simply dotnet new webapi Add middleware as describved in readme [HttpGet] public IActionResult Get() { return BadRequest("wadwadwd"); }

Does not return a ProblemDetail response. First frsutrated because I thought middleware doesn't support 400s, but it seems that's just broken now..

Btw: options.IsProblem = context => { return context.Response.StatusCode >= 400; };

doesn't fix it..

[khellang]

Duplicate of https://github.com/khellang/Middleware/issues/65. Short answer; you're returning a body ("wadwadwd"), which is sent instead of the problem details response.

[SoftwareDreamer]

ok, thanks..

Un-intuitive to say the least. The whole promise of ProblemDetails was "you enable it and any error now becomes this new nice W3C format". I think it was badly communictaed, as I don't see how to switch it "just on" without rewriting all teh controllers in an existing application. Is there a way to "inject" the problem details? Seems no from the linked issue.

[SoftwareDreamer]

Ok, IETF standard format, whatever :) https://tools.ietf.org/html/rfc7807

I don't understand why the "unintuitivesness" isn't clear.. when BadRequest() without a string is used, it returns a ProblemDetail object. The moment you sue an "explainer" text it doesn't work anymore.. from a users perpective that seems like a bug, and is certainly unexpected. In other words: it should mentioned in the README

From an internal perspective it might make sense once one understands all the parts. But then still it seems to me (I used Middleware enough time that I thought I understand what's going on, but seems I was wrong) that a middleware should be able to somehow intercept a response before it's written to the body. Probably wrong assumption there.

[khellang]

Un-intuitive to say the least. Is there a way to "inject" the problem details? Seems no from the linked issue.

Did you read the linked issue? It seems you've fundamentally misunderstood how the ASP.NET Core middleware pipeline works. When someone starts writing to the response body, no other middleware should write to it, otherwise you'd end up with garbled content.

I don't see how to switch it "just on" without rewriting all teh controllers in an existing application

There's no magic here. If you return content back to the client, you'll send that content back to the client. What did you expect the middleware to do with your existing content? Overwrite it? Add to it? You could probably turn on buffering for the entire response (which would introduce a non-trivial performance hit), but I still don't understand what a general purpose middleware would do with that body.

The whole promise of ProblemDetails was "you enable it and any error now becomes this new nice W3C format".

I don't think that was the promise and it's not a W3C format :smile:

[khellang]

but why is there an options.MapStatusCode(..) method then?

So it can transform empty responses with an error status code, i.e. 401, 403, 404, 500, 502 etc.

[SoftwareDreamer]

reading #65 again it seems that there is a lower level way of doing it, I will look into it.. right now this is too brittle for us to adapt, as simple parameter changes mean changes in payload, breaking clients.. will see what I can come up with.

[khellang]

The moment you sue an "explainer" text it doesn't work anymore.. from a users perpective that seems like a bug, and is certainly unexpected.

But it's not an "explainer text", it's the entire error object. From the documentation:

error - An error object to be returned to the client.

It's the error object, not an addition. I ask again; what did you expect to happen to the "explainer text"?

[khellang]

right now this is too brittle for us to adapt, as simple parameter changes mean changes in payload, breaking clients..

This is fundamentally how MVC works. If you add an error object, it will return it. If you don't, you'll return an empty response 😂 If anything, this is an issue for the ASP.NET team to fix MVC.

[SoftwareDreamer]

you are of course right.. I would expect the "string error object" to be intercepted and converted to a ProblemDetails object, as I thought the middleware would do that, as naive as that might sound to you, that is the expectation I came in with.

In other words: whatever gets returned, it would convert the object to a problem details object (string becomes Detail directly, ProblemDetail handled in a special way, everything else probably JSON serialized, as it represents custom error details). Might sound naive from an implementors perspective, but it would seem un-surprising from a users perspective

[SoftwareDreamer]

right now this is too brittle for us to adapt, as simple parameter changes mean changes in payload, breaking clients..

This is fundamentally how MVC works. If you add an error object, it will return it. If you don't, you'll return an empty response 😂 If anything, this is an issue for the ASP.NET team to fix MVC.

I do understand how MVC works, used it for a while. ;) I do not understand how your middleware works, and apparently not what middlewares are capable of. Again, will look into what's possible with your mentioned " You could probably write some MVC filters to tweak the behaviors, but I wanted a lower level middleware that works (more or less) without MVC."

[khellang]

Right, but that's just not how the middleware pipeline works. By the time the response gets back to the middleware, MVC has already serialized the error object you passed in and flushed it to the client. The middleware has no knowlege about MVC (or whatever other framework/library you have in your pipeline). Its hands are tied at that point.

Say that I turned on buffering for all responses that pass through the middleware. First of all that would force all users to buffer all of their request, which is a non-starter already, but let's just go along with it... That would allow anyone downstream to write to the body stream, without actually flushing the content to the client, which in turn would allow the middleware to inspect the response object. What do you propose the middleware would do with that in-memory stream of bytes if the status code is >= 400?

[SoftwareDreamer]

Right, but that's just not how the middleware pipeline works. By the time the response gets back to the middleware, MVC has already serialized the error object you passed in and flushed it to the client. The middleware has no knowlege about MVC (or whatever other framework/library you have in your pipeline). Its hands are tied at that point.

Good to know; so that would really be something for the MVC team, but maybe there weere back compat issues.. Also I saw that 3.0 adds Problem() as a method.. doesn't help old codebases but is maybe our solution for the future.

Side note: maybe a simple sentence what your middleware actually does would be good to understand in Readme, like "it only intercepts exceptions and transforms them to ProblemDetails in addition to what MVC ProblemDetails implementation does" could help here..

Say that I turned on buffering for all responses that pass through the middleware. First of all that would force all users to buffer all of their request, which is a non-starter already, but let's just go along with it... That would allow anyone downstream to write to the body stream, without actually flushing the content to the client, which in turn would allow the middleware to inspect the response object. What do you propose the middleware would do with that in-memory stream of bytes if the status code is >= 400?

Well... I'm a big fan of perf-orenited design and I saw Fowlers remarks about this, no buffering this would be hideous. You mentioned in #65 " You could probably write some MVC filters to tweak the behaviors, but I wanted a lower level middleware that works (more or less) without MVC." I will try this instead, maybe that direction would be the "correct" one for what I need. It seems a middleware can really not do a lot here. It's just unfortunate from a consumers perpective that the MVC team was non-consistent with the feature.

To answer your question: as mentioned above, whatever thingie that intercepts the 400 return, it would transform it to problem details object via the logic mentioned above. But buffering is not the solution..

[khellang]

Also I saw that 3.0 adds Problem() as a method.. doesn't help old codebases but is maybe our solution for the future.

That doesn't really bring anything to the table compared to this middleware. You still have to instantiate a ProblemDetails object to return.

Side note: maybe a simple sentence what your middleware actually does would be good to understand in Readme, like "it only intercepts exceptions and transforms them to ProblemDetails in addition to what MVC ProblemDetails implementation does" could help here..

Yes, documentation as a whole is missing, so it isn't very helpful currently 😂

I will try this instead, maybe that direction would be the "correct" one for what I need. It seems a middleware can really not do a lot here. It's just unfortunate from a consumers perpective that the MVC team was non-consistent with the feature.

Right, you could also look into the built-in handling for this in MVC 2.2. But that only handles client-errors AFAIK.

[SoftwareDreamer]

Ok I think the right way to achieve what I need is to create a Result filter: -https://docs.microsoft.com/en-us/aspnet/core/mvc/controllers/filters?view=aspnetcore-2.2#result-filters and inject that globally in Confgure() in addition to your middleware.. I think that could work.

The context: we are in the process of creating "standard" templates and I needed a solution to have consistent ProblemDetail returns no matter what the developer returns (as long as it's a status code >= 400)

Thanks for all the help so far! Much appreciated..

[khellang]

Yes, you could add a result filter, but I'm curious how you'd handle the arbitrary return value from the result (without just throwing it away). I'd love to see it if you get something working.

[SoftwareDreamer]

Also I saw that 3.0 adds Problem() as a method.. doesn't help old codebases but is maybe our solution for the future.

That doesn't really bring anything to the table compared to this middleware. You still have to instantiate a ProblemDetails object to return.

From the blog post: return Problem(title: "An error occurred while processing your request", statusCode: 400);

if that's correct, it's already close enough to return BadRequest("An error occurred while processing your request"); for me..

Regarding the arbitrary return value.. I'll see :) Just has to work for us internally, tbh. I can cut corners if neccessary ^^ But I imagine it shouldn't be too hard handle anything that's not a ProblemDetail (which is handled correctly already) by special-casing some builtin types like string and trying to JSON-serializing the rest.

[khellang]

if that's correct, it's already close enough

This isn't that far off either (of course, most of that info is optional);

https://github.com/khellang/Middleware/blob/c454363a0684ce6bbb47c12835d2024db749e30e/samples/ProblemDetails.Sample/Program.cs#L123-L133

[SoftwareDreamer]

Sure, but Problem(..) seems like a more concise format, and not having to instantiate an object (visibly) pushs it over the acceptance-bar for me. I guess the MVC team probably wouldn't have added it, without feedback in this regard.

[SoftwareDreamer]

Man this thread got long, I love Github "chat", but don't wanna keep you any longer ;) I'll post when I have a solution that meets our bar regarding the ResultFilter.. thanks so far!