fix(backend): handle undefined array key name#1844
fix(backend): handle undefined array key name#1844Baraka24 wants to merge 2 commits intocypht-org:masterfrom
Conversation
modules/core/hm-mailbox.php
Outdated
| } | ||
| $result = $this->connection->get_folder_status($folder); | ||
| return $result['name']; | ||
| return $result['name'] ?? null; |
There was a problem hiding this comment.
This doesn't handle the issue properly but simply ignores it. Why is the name not defined? Shouldn't a folder always have a name?
If the ajax call requests non-existing folder, then trace back the code to see why does it do it? Ultimately, if it is a user error (e.g. setting up custom folders for something that doesn't exist), we should show an error to the user, so they can fix. If it is Cypth code handling data incorrectly, we should fix the code error.
There was a problem hiding this comment.
We discovered that an exception is normally thrown at line 182 in the "modules\imap\hm-ews.php" file, but unfortunately its $e->getMessage() is empty, making it impossible to capture the real error behind this behavior. This explains why the get_folder_status() method returns an empty result array in the catch block, which then causes the "undefined array key 'name'" error in the get_folder_name() method - the EWS API is failing silently without providing any meaningful error message, so we need to enhance the exception handling to log more details like the exception type, code, and stack trace to properly diagnose what's causing the folder lookup to fail.
There was a problem hiding this comment.
Then, go for it and debug further the EWS client or API. There should be an underlying issue somewhere. I don't like the current approach. Folder not found means something - if it is a normal folder name, then this is a real issue to debug further. Note that EWS and the client we use is a bit pesky about folder naming having both constants, names and base64 encoded IDs with various ways to search for a folder.
013ed34 to
96e63a4
Compare
🍰 Pullrequest
Issues
Todo