inertiajs / inertia-rails

The Rails adapter for Inertia.js.
https://inertiajs.com
MIT License
529 stars 43 forks source link

Fix error handling by returning original HTTP status #42

Closed ledermann closed 3 years ago

ledermann commented 4 years ago

Currently, an InertiaRails response has always the status code 200. This seems wrong to me, because if there was an error (like 404 or 500), the corresponding status code should be returned, not 200.

The Laravel adapter does the same thing, but the docs recommends overriding this behavior in the application - from what I understand.

This thing was discussed for the Laravel adapter some time ago. In Rails, error handling is different than in Laravel, so in my opinion returning the original status code should be returned. This fix allows handling errors I am doing in the PingCRM demo application.

Demo using the PingCRM for Rails demo application:

ledermann commented 3 years ago

Hi @bknoles and @BrandonShar, what do you think? IMHO this is a simple fix for an obvious bug.

bknoles commented 3 years ago

@ledermann does this conflict with #50 at all (conceptually)?

@reinink could you weigh in on whether returning 404s (or similar statuses) instead of 200s for error statuses fits into Inertialand ok?

ledermann commented 3 years ago

@bknoles I don't see any conflicts with #50. Also, the tests are green after merging both branches together. No issues from my side.