fixes #58 handle ActionDispatch::Http::Parameters::ParseError #59
Conversation
* when a request contains unparsable JSON, it would fail with a 500 with ActionDispatch::Http::Parameters::ParseError even before evaluating routes * created a test that reproduces this error (returned 500 instead of 404) * catching this error early, and the test is now green
|
Thanks! LGTM |
|
Thank you @yuki24 ! did you / can you push a new release with this fix? |
|
also a question: could there be other errors that need to be explicitly handled? I'm not sure I fully understand when an error is raised before it even reaches the rails app itself, but I can imagine there's a chance of some other exceptions being thrown at that (mysterious) point :) so would be good to catch those as well? |
It is very hard to know all the errors that need to be handled beforehand. Just like, aircraft manufactures do their very best so airplanes would never ever crash. However, while it is a very rare event, airplanes do crash. This is a sad truth, but it's impossible to save all potential crashes even before flying.
It would be interesting to learn about the rack middleware stack. The
Unfortunately I'm a bit busy today so the earliest can make is tomorrow night. I would like to do a few things such as switching over to GitHub Actions from Travis CI, testing against Rails 6.2, Ruby 3.0, etc, so it may take time to get the whole things sorted out. |
|
Thank you @yuki24 That's an interesting analogy :) although I would have hoped airplanes have better error controls ;-)
Yes, I was thinking the same thing. I can give it a try if I find some time. I don't have that much experience with github actions, but I hope I can figure something out. Btw, maybe it's also a good opportunity to deprecate support for some older ruby/rails versions? :) |
|
@yuki24 I started working on github actions matrix ... however it actually uncovered an issue with this code change. See https://github.com/gingerlime/rambulance/pull/1/checks?check_run_id=3536230995 - in older rails versions there's no |
|
Ok, I found a workaround ... not sure if it's elegant, but it seems to work. Take a look at gingerlime#1 most jobs seem to pass but not sure about ruby-head however ... |
|
I should be able to wrap it up in a PR with the info you provided. Thanks @gingerlime for your patience. |
|
perfect, thank you @yuki24. Let me know if there's anything else I can do to help. Also curious to know if some older versions of rails/ruby can be deprecated by the Gem. It could simplify the code and tests and make maintenance easier going forward. |
* based on https://dev.to/pikachuexe/ruby-gems-ci-using-github-action-136o * note that there's no direct equivalent to allow_failures see https://dev.to/pikachuexe/ruby-gems-ci-using-github-action-136o so there's only a workaround, which requires you to include specific combinations of the matrix ... * removed old .travis.yml * updated and fixed code and specs for yuki24#59 * TODO?: - [ ] check that coverage is similar to travis - [ ] refresh ruby / rails versions - [ ] deprecate some older versions
* based on https://dev.to/pikachuexe/ruby-gems-ci-using-github-action-136o * note that there's no direct equivalent to allow_failures see https://dev.to/pikachuexe/ruby-gems-ci-using-github-action-136o so there's only a workaround, which requires you to include specific combinations of the matrix ... * removed old .travis.yml * updated and fixed code and specs for #59 * TODO?: - [ ] check that coverage is similar to travis - [ ] refresh ruby / rails versions - [ ] deprecate some older versions
* based on https://dev.to/pikachuexe/ruby-gems-ci-using-github-action-136o * note that there's no direct equivalent to allow_failures see https://dev.to/pikachuexe/ruby-gems-ci-using-github-action-136o so there's only a workaround, which requires you to include specific combinations of the matrix ... * removed old .travis.yml * updated and fixed code and specs for #59 * TODO?: - [ ] check that coverage is similar to travis - [ ] refresh ruby / rails versions - [ ] deprecate some older versions
|
This has been released as 2.0.0. Sorry for the long wait! |
with ActionDispatch::Http::Parameters::ParseError even before
evaluating routes
404)