Email::Processor does not expect to be called with mail=nil

thoka · March 16, 2023, 9:31pm

My error-log is full of these messages:

Job exception: undefined method `is_bounce?’ for nil.

Backtrace:

/var/www/discourse/lib/email/processor.rb:27:in `rescue in process!' 
/var/www/discourse/lib/email/processor.rb:16:in `process!' 
/var/www/discourse/lib/email/processor.rb:13:in `process!' 
/var/www/discourse/app/jobs/scheduled/poll_mailbox.rb:23:in `process_popmail' 
/var/www/discourse/app/jobs/scheduled/poll_mailbox.rb:43:in `block (2 levels) in poll_pop3' 
net-pop-0.1.2/lib/net/pop.rb:669:in `each' 
net-pop-0.1.2/lib/net/pop.rb:669:in `each_mail' 
/var/www/discourse/app/jobs/scheduled/poll_mailbox.rb:40:in `block in poll_pop3' 
net-pop-0.1.2/lib/net/pop.rb:531:in `start' 
/var/www/discourse/app/jobs/scheduled/poll_mailbox.rb:39:in `poll_pop3' 
/var/www/discourse/app/jobs/scheduled/poll_mailbox.rb:14:in `execute' 
/var/www/discourse/app/jobs/base.rb:249:in `block (2 levels) in perform' 
rails_multisite-4.0.1/lib/rails_multisite/connection_management.rb:80:in `with_connection'
/var/www/discourse/app/jobs/base.rb:236:in `block in perform' 
/var/www/discourse/app/jobs/base.rb:232:in `each' 
/var/www/discourse/app/jobs/base.rb:232:in `perform' 
/var/www/discourse/app/jobs/base.rb:297:in `perform' 
mini_scheduler-0.15.0/lib/mini_scheduler/manager.rb:122:in `process_queue' 
mini_scheduler-0.15.0/lib/mini_scheduler/manager.rb:70:in `worker_loop' 
mini_scheduler-0.15.0/lib/mini_scheduler/manager.rb:59:in `block (2 levels) in ensure_worker_threads'

I’m wondering, how @receiver could get nil in

github.com

discourse/discourse/blob/84f590ab83b90d4c6754e247744567f75274fb69/lib/email/processor.rb#L17-L27


      
          begin
            @receiver = Email::Receiver.new(@mail, @opts)
            @receiver.process!
          rescue RateLimiter::LimitExceeded
            if @opts[:retry_on_rate_limit]
              Jobs.enqueue(:process_email, mail: @mail, source: @opts[:source])
            else
              raise
            end
          rescue => e
            return handle_bounce(e) if @receiver.is_bounce?

thoka · March 19, 2023, 5:36am

My current understanding is, that Email::Processor.process! calls Email::Receiver.new with mail=nil which raises an Email::Receiver.EmptyEmailError, leaving @receiver undefined.

The following rescue part expects an initialized @receiver.

Shouldn’t Email::Processor.initialize raise some Error when called with mail==nil?

sam · March 20, 2023, 5:19am

My guess is that somehow is exploding.

I think a PR that changes:

@receiver.is_bounce? to @receiver&.is_bounce? would be fine, at least the error would float into handle_bounce.