Bad message number

[Lewiscowles1986]

PHP Warning: imap_fetchbody(): Bad message number in /projects/CODESIGN2/email-harvest/vendor/ddeboer/imap/src/Message/Part.php on line 334

I attached a debugger to get the following message numbers from src/Mailbox.php#L57

public function getMessages(SearchExpression $search = null)

[ 
    1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
    11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
    21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
    31, 32, 33, 34, 35, 36, 37, 38, 39, 40,
    41, 42, 43, 44, 45, 46, 47, 48, 49, 50,
    51, 52, 53, 54, 55, 56, 57, 58, 59, 60,
    61, 62, 63, 64, 65, 66, 67, 68, 69, 70,
    71, 72, 73, 74, 75, 76, 77, 78, 79, 80,
    81, 82, 83, 84, 85, 86, 87, 88, 89, 90,
    91, 92, 93, 94, 95, 96, 97, 98, 99, 100,
    101, 102, 103, 104, 105, 106, 107, 108, 109, 110,
    111, 112, 113, 114, 115, 116, 117, 118, 119, 120,
    121, 122, 123, 124, 125, 126, 127, 128, 129, 130,
    131, 132, 133, 134, 135, 136, 137, 138, 139, 140,
    142, 143, 144, 145, 146, 147, 148, 149, 150,
    151, 152, 153, 154, 155, 156, 157, 158, 159, 160,
    161, 162, 163, 164, 165, 166, 167, 168, 169, 170,
    171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 
    181, 182
]

It looks like the email host (dreamhost) cannot be trusted to return ID's that return (seems 141 is missing and 182 is there, which means it's probably skipping a message).

What (sh|c)ould be the resolution?

[Lewiscowles1986]

Btw this can return a caught exception using the following code added to src/Message/Part.php

    //
    // within class, also needs
    // use Ddeboer\Imap\Exception\PartDoesNotExist;
    //
    private function partDoesNotExistSetHandleAndExecute(callable $c) {
      set_error_handler(
          function ($nr, $error) {
              throw new PartDoesNotExist(
                  $this->messageNumber,
                  $error
              );
          }
      );
      $result = $c();
      restore_error_handler();
      return $result;
    }

Exception Class

<?php

namespace Ddeboer\Imap\Exception;

class PartDoesNotExist extends Exception
{
    public function __construct($number, $error)
    {
        parent::__construct(
            sprintf(
                'Part %s does not exist: %s',
                $number,
                $error
            )
        );
    }
}

[Lewiscowles1986]

This is part of #146 but method signature from issue should be (using exception error catcher here)

    public function getRAW() {
      return $this->partDoesNotExistSetHandleAndExecute(function() {
        return imap_fetchbody(
          $this->stream,
          $this->messageNumber,
          "",
          \FT_PEEK
        );
      });
    }

[Slamdunk]

This library uses only message uid: if you deleted an old message, the id of newer messages remain the same, so it is ok to have missing numbers. You need to use \FT_UID constant.

Note that this functionality will be present in the next release, look at #146

[Lewiscowles1986]

This isn't a duplicate, I've just tried setting the \FT_UID constant.

Mailbox exception! Message: Part 182 does not exist: imap_fetchbody(): Bad message number

[Lewiscowles1986]

Also that merge should try using null or "" as the third param, so that we can still peek at messages in 4th param 😢

[Slamdunk]

I'm sorry we can't help you if you have custom source code.
Please try to use the latest version of this library downloaded from the develop branch (e.g. with the ZIP).

Also that merge should try using null or "" as the third param, so that we can still peek at messages 😢

It does, take a look at the merge commit: 6ae87c9

[Lewiscowles1986]

The error I'm reporting is not from custom code, it's from an existing method in this library. I'm not asking you to fix, I'm recording a bug that exists in this library, even with the \FT_UID instead of \SE_UID

I've also been recording details in-case I get busy or don't have time to fix, so the next person that comes along can fix, and not have to duplicate my work.

As for the PR, I see the additional commit, thanks for adding. I couldn't know it existed though could I? It's been 28 minutes since it was committed and not part of the PR. This is not an attack, or a criticism, please stop treating it as such.

[Slamdunk]

The error I'm reporting is not from custom code, it's from an existing method in this library.

The method you pasted:

    public function getRAW() {
        // [...]
    }

Isn't part of this library. If you think it is, please post the github link of the file that contains these lines.

I've also been recording details in-case I get busy or don't have time to fix, so the next person that comes along can fix, and not have to duplicate my work.

Each feedback is welcome. At the same time we can fix only issues that are reproducible: without a data set that reproduces the error we can't alter the code because we can't prove the fix would be valid.

As for the PR, I see the additional commit, thanks for adding. I couldn't know it existed though could I?

Unfortunately not: as a Collaborator I can push commits to a PR only during the merge, and it is not very noticeable in Github this step 😞

[Lewiscowles1986]

without a data set that reproduces the error we can't alter the code because we can't prove the fix would be valid.

I don't need you to fix though, and the error still exists.

The error is not in the getRAW call btw, the same error comes from the doGetContent method for these messages.

The code I posted allows the PHP Warning to be converted into an exception (following convention from elsewhere in this lib).

This will also help with a test-case for the existing method to get a specific part from a message for text and HTML (that exception wouldn't be elsewhere if nobody encountered a missing message). I'm not saying it has to be in the lib (which is why it's in an issue, not a PR).

Issues are not all created to ask you to solve. They document edge cases, and by closing this one, it makes it harder to see it's still a current issue with the lib as-is in a release or master branch (apparently the develop branch too).

It's one of those weird edge cases and could even be related to the version of IMAP PHP extension supports (cantfix or wontfix)

One way to validly remove this would be to remove the coupling on php-ext-imap, then the library is not at fault (it's only at fault vis-a-vis the php-ext-imap by the looks of things anyway).

[Slamdunk]

As far as I can see, this library will always be just a wrapper to the imap extension. Writing raw streams (like roundcube does) would be too much for our commitment to this project.

I am sorry but we can't put efforts in issues that cannot be reproduced: having them hanging here forever wouldn't help much in my humble opinion.

[Lewiscowles1986]

Again, I'm not asking you to fix it. (It's been an unreported issue since 2015, nobody fixed that).

The reason I mentioned separating was not to ask you to do it, it was to add a written record of a way to solve. This isn't about solving, it's about documenting what-is.