[dev] [commits] Horde branch FRAMEWORK_5_0 updated. 00b796b064a73a3429775abccd7e41aba253fc2a

[Michael J Rubinsky]

Quoting Thomas Jarosch <thomas.jarosch at intra2net.com>:

> On Thursday, 18. April 2013 11:15:47 Michael J Rubinsky wrote:
>> >>  
>> http://git.horde.org/horde-git/-/commit/00b796b064a73a3429775abccd7e41aba253fc2a
>> >
>> > Regarding the exception handler, what about adding a debug log?
>>
>> It's logged when the calling code catches the exception. E.g., see
>> Horde_ActiveSync_Sync#syncronize()
>
> Ok. I still don't get why Horde_ActiveSync_Imap_Adapter:getMessages()
> catches the exception and hides it.

In general, I try to avoid outputting the same error/notice/whatever  
in the log multiple times. Horde_Core_ActiveSync_Driver#getMessage()  
(or more accurately Horde_ActiveSync_Driver_Base#getMessage()) has to  
throw Horde_Exception_NotFound() when no message is returned. It  
throws this when the single requested message is not returned.

Before that commit, we were already checking for the message not being  
found, but only during the initial IMAP search for the message. It's  
also possible that the message vanishes in the milliseconds between  
getting the initial IMAP search result, and attempting to build the  
proper message structure based on the ActiveSync bodyprefs and such.  
That is what this latest commit checks.

Anyway, after that long winded explanation, if you feel strongly about  
adding logging there, it's fine, just submit a pull request, or patch  
or whatever. Word the log along the lines of "message vanished after  
loading, but before building activesync message" or similar.


> If the code is refactored later on or getMessages() gets re-used,
> we might miss the information that something was wrong.
>
> Can't we log it "more local" at the place the exception is caught?

> Thanks for your patience :)

No worries. It's extremely helpful to have another set of eyes looking  
at, and more importantly, testing the code like you have been doing.  
Thanks!
-- 
mike

The Horde Project (www.horde.org)
mrubinsk at horde.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-keys
Size: 2200 bytes
Desc: PGP Public Key
URL: <http://lists.horde.org/archives/dev/attachments/20130419/68d66b9a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 6062 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.horde.org/archives/dev/attachments/20130419/68d66b9a/attachment-0001.bin>