sj26 / mailcatcher

Catches mail and serves it through a dream.
http://mailcatcher.me
MIT License
6.3k stars 575 forks source link

make it work with ruby 3.1 by upgrading thin to the latest version … #496

Closed Wanderfalke closed 2 years ago

Wanderfalke commented 2 years ago

…and replacing skinny with faye-websocket.

also allow mailcatcher process to be stopped with SIGINT / CTRL+C

Vincent14 commented 2 years ago

Hi @sj26 or @tagliala @westonganger could you have a look please? <3

westonganger commented 2 years ago

Unfortunately I am not a maintainer of this project. I think I reached out before. But if more maintainership help is needed I am willing.

sj26 commented 2 years ago

Thanks for the contribution! But there is already a conflicting stategy for upgrading thin and websockets etc over here: https://github.com/sj26/mailcatcher/pull/467

The blocker is testing, including adding more feature spec coverage. That blocker still exists on this branch. If you're keen to contribute, looking at the surface area of mailcatcher and adding feature specs would be a great contribution.

sj26 commented 2 years ago

allow mailcatcher process to be stopped with SIGINT / CTRL+C

This should already be working:

image
sj26 commented 2 years ago

Reading this again, it's a pretty small intervention so might be easier to land. I'll do a review and maybe we can land it.

Wanderfalke commented 2 years ago

@sj26 Thank you for taking time to review my PR. I addressed your comments. Please have another look when you can.

sj26 commented 2 years ago

I've added a spec on main which tests that mailcatcher exits cleanly on interrupt, and rebasing this branch to include that spec causes a failure:

  1) Quit quits cleanly on Ctrl+C
     Failure/Error: expect(status).to be_exited
       expected `#<Process::Status: pid 56232 SIGINT (signal 2)>.exited?` to be truthy, got false
     # ./spec/quit_spec.rb:41:in `block (2 levels) in <top (required)>'
shaneshort commented 2 years ago

any particular reason this wasn't merged? I'm currently unable to install this on ruby 3.x and I suspect this PR would help.

sj26 commented 2 years ago

There is a failing spec rebased on main, and not enough testing generally to be sure it hasn't broken anything.

sj26 commented 2 years ago

Hi folks! Thanks for this work here, especially @Wanderfalke.

Ruby 3.1 support and a switch to faye-websocket are available in 0.9.0.beta1. I've love some folks to test. You can install the latest pre-release with:

gem install --pre mailcatcher

I think there are still some quirks. I don't think the websockets send the quit message before shut down. But that's not critical.

I'm keen to upgrade Thin, too, but I'd like to bed in the switch to faye, first. Beyond that I'll see if we can complete the switch to the async gems (#467).

larryh commented 1 year ago

I know this is marked as Closed (so hopefully I'm not wasting my time typing this), but I wanted to give the maintainers and anyone else who has this problem some feedback...

I'm using Rails 3.1.0, and when I installed and tried to run mailcatcher 0.9.0 this is what I got:

Unable to load the EventMachine C extension; To use the pure-ruby reactor, require 'em/pure_ruby'
:85:in `require': libssl.so.1.1: cannot open shared object file: No such file or directory - /home/larry/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/gems/eventmachine-1.0.9.1/lib/rubyeventmachine.so (LoadError)
        from :85:in `require'
        from /home/larry/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/gems/eventmachine-1.0.9.1/lib/eventmachine.rb:8:in `'
        from :85:in `require'
        from :85:in `require'
        from /home/larry/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/gems/mailcatcher-0.8.1/lib/mail_catcher.rb:7:in `'
        from :85:in `require'
 

So I ran the following two commands:

gem uninstall mailcatcher
gem install --pre mailcatcher

And it worked like a charm, specifically:

larry@system76-pc:~/RubymineProjects/BackEnd$ mailcatcher
Starting MailCatcher v0.9.1.beta1
==> smtp://127.0.0.1:1025
==> http://127.0.0.1:1080
*** MailCatcher runs as a daemon by default. Go to the web interface to quit.

Thanks for the great gem!

sj26 commented 1 year ago

Sorry @larryh, that's very weird!

That sounds like a problem within eventmachine. Perhaps it linked to an older version of openssl? Are you using homebrew? Or another system package manager? It might have upgraded shared libraries beneath your feet.

Reinstalling the gems usually fixes it up for me:

gem pristine eventmachine
# or
gem pristine --all
larryh commented 1 year ago

Hi, Thanks for the constant feedback - I know you must be busy and it is much appreciated.

A "gem list" command tells me I have Eventmachine 1.2.7, which I believe is the latest version.(From 5 years ago!) It's not in my (Rails project) Gemfile or even Gemfile.lock. Also, I don't think I ever intentionally installed it, so maybe it just comes along for the ride when I install Mailcatcher.

Also, I'm not on a Mac, so no homebrew. (I'm on Ubuntu)

I'll just continue to refresh the page to see new emails.

Thanks again, Larry

sj26 commented 1 year ago

Heh, Homebrew runs on Linux these days.

But even apt may have bumped your libssl. And apt isn't aware of rubygems, so wouldn't cause dependent gems to be recompiled.

I'd recommend a gem pristine --all just to be safe :-)

larryh commented 1 year ago

Thanks for the tip. I did a "gem pristine eventmachine" but that didn't do anything; I still have to refresh the page to see the email.

I don't want to try the "all" command because I'm gun-shy of potential unintended consequences. Oftentimes (or at least sometimes) issuing commands that sweeping can result in spending hours just trying to get back to where you were, i.e. "when things worked."

I'll just stick with refreshing the page when I need to.

Thanks for all of your help!