search

joeyates / imap-backup

Backup and Migrate IMAP Email Accounts
MIT License
1.33k stars 74 forks source link

When restoring, error exit when local backup contains a message reported by server as TOOBIG #120

Closed svetzal closed 2 years ago

svetzal commented 2 years ago

Everything was working great for me until this - I have a bunch of mailboxes to move off Google and onto iCloud this weekend.

The message is persistent, it simply gets to this message in the backup data and bails every time, which blocks the rest of the restore.

/home/svetzal/.rvm/rubies/ruby-3.1.2/lib/ruby/gems/3.1.0/gems/net-imap-0.2.3/lib/net/imap.rb:1247:in `get_tagged_response': Cannot upload message, TOOBIG (took 0 ms) (Net::IMAP::BadResponseError)                 
        from /home/svetzal/.rvm/rubies/ruby-3.1.2/lib/ruby/gems/3.1.0/gems/net-imap-0.2.3/lib/net/imap.rb:1299:in `block in send_command'                                                                           
        from /home/svetzal/.rvm/rubies/ruby-3.1.2/lib/ruby/3.1.0/monitor.rb:202:in `synchronize'                                                                                                                    
        from /home/svetzal/.rvm/rubies/ruby-3.1.2/lib/ruby/3.1.0/monitor.rb:202:in `mon_synchronize'                                                                                                                
        from /home/svetzal/.rvm/rubies/ruby-3.1.2/lib/ruby/gems/3.1.0/gems/net-imap-0.2.3/lib/net/imap.rb:1281:in `send_command'                                                                                    
        from /home/svetzal/.rvm/rubies/ruby-3.1.2/lib/ruby/gems/3.1.0/gems/net-imap-0.2.3/lib/net/imap.rb:753:in `append'                                                                                           
        from /home/svetzal/.rvm/rubies/ruby-3.1.2/lib/ruby/3.1.0/forwardable.rb:238:in `append'                                                                                                                     
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/account/folder.rb:94:in `append'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/uploader.rb:24:in `block in run'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/serializer/mbox_store.rb:96:in `block in each_message'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/serializer/mbox_enumerator.rb:20:in `block (2 levels) in each'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/serializer/mbox_enumerator.rb:15:in `loop'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/serializer/mbox_enumerator.rb:15:in `block in each'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/serializer/mbox_enumerator.rb:12:in `open'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/serializer/mbox_enumerator.rb:12:in `each'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/serializer/mbox_store.rb:92:in `with_index'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/serializer/mbox_store.rb:92:in `each_message'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/uploader.rb:16:in `with_index'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/uploader.rb:16:in `run'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/account/connection.rb:170:in `restore_folder'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/account/connection.rb:97:in `block in restore'
        from /home/svetzal/.rvm/gems/ruby-3.1.2/gems/imap-backup-5.2.0/lib/imap/backup/account/connection.rb:91:in `block in local_folders'
        from <internal:dir>:220:in `glob'

I'm looking at uploader.rb line 24, and might just set it inside a rescue block and find a way to collect UIDs that didn't upload to report to the user at the end.

Just wondering if you've already got thoughts on how you might address this.

joeyates commented 2 years ago

Hi @svetzal

Thanks for opening the issue.

I think this could be handled in a similar way to download errors:

  1. a retry_on_error(errors: [Net::IMAP::BadResponseError]) in the append function in folder.rb, returning nil if the call fails a couple of times,
  2. logging of any failed appends in the upload_message function in upload.rb.
svetzal commented 2 years ago

I think the TOOBIG message is probably an example where a retry would not be appropriate, if I get no other errors I'll see if I can nicely express a way to do retries around anything else. I'm awaiting a restore of about 45,000 messages right now.

Thanks for the direction. I have a dirty hack in right now in my fork, just to see what kinds of errors I might see, but I'll work on some kind of PR after the weekend.

joeyates commented 2 years ago

In imap-backup, exceptions are only distinguished by their class. Basing behaviour on the message attribute containing "TOOBIG" would be out of keeping with the rest of the application.

I would handle it this way:

A single generic retry for Net::IMAP::BadResponseError handles other types of failures caused by transitory problems, but also handles "TOOBIG", which of course is not transitory. A Logger.logger.debug of the error message will allow the user to understand the exact cause of failure, while in upload.rb we can report any skipped message UIDs with Logger.logger.warn.

This will mean that, without debug logging enabled, the skipped message will show up, and will allow fuller info if the user wants to dig into the problem with debug logging.

svetzal commented 2 years ago

Hmm, it's been a few years since I was doing ruby full-time :) but I have something I'm reasonably happy with - just seeing how it works in practice.

I realize this is a lot like coding by remote control, but given the above guidance, what kind of testing would you like to see on this? I'm snooping around but coming up dry on sad-path checks like what I'm thinking.

Thanks for indulging me, I'm enjoying my foray back into ruby, I miss it.