search

franzliedke / whoops-middleware

PSR-15 compatible middleware for Whoops, the pretty error handler
MIT License
28 stars 10 forks source link

Responding to requested content type #11

Closed MrHash closed 4 years ago

MrHash commented 4 years ago

At the moment the library always returns an HtmlResponse for the Whoops handler. I made a workaround which sets a default content type for the response but this is a bit of a hack as it should probably insert best fit according to the request accept header. Please consider handling, but if you want a PR in the meantime let me know.

https://github.com/MrHash/whoops-middleware/commit/e4f65776719a1ca66d8a3df1cadc3406cd1a3602

franzliedke commented 4 years ago

Hey, thanks for the issue!

I am a bit confused - isn't this what we're already doing with this block: https://github.com/franzliedke/whoops-middleware/blob/376732256db67f9c342b5c36541366c5d05d268f/src/WhoopsRunner.php#L39-L63

Or are you merely saying that the corresponding Content-Type header needs to be added to the response as well? It seems like that was indeed forgotten.

MrHash commented 4 years ago

Yes as you say, the response should have the correct content type. In the linked commit i have implemented a simple solution which picks the first content-type based on the preferred format, which is wrapped in a TextResponse.

franzliedke commented 4 years ago

Thanks for the report!

I implemented your fix, and then had some fun refactoring the entire content negotiation part to make this more pretty.

Will release a new version as well.

MrHash commented 4 years ago

Ok that's great. Not sure if you're familiar but this package has some pretty comprehensive negotiation handling https://github.com/willdurand/Negotiation

franzliedke commented 4 years ago

Thanks for the pointer. If other negotiation-related issues come up, I might give it a try.