Closed Bug 1697075 Opened 8 months ago Closed 8 months ago

messages.getFull don't reject upon failure

Categories

(Thunderbird :: Add-Ons: Extensions API, defect)

defect

Tracking

(thunderbird_esr78 fixed)

RESOLVED FIXED
88 Branch
Tracking Status
thunderbird_esr78 --- fixed

People

(Reporter: benc, Assigned: darktrojan)

References

Details

Attachments

(2 files)

(This arose as Part of Bug 1644027, but isn't the cause. You can find steps to reproduce over there - there's a simple extension posted in the first few comments).

messages.getFull() and messages.getRaw() are failing (on news messages), but not causing an exception/rejection/whatever.

Just looking at getFull(), it calls MsgHdrToMimeMessage(), which is throwing an exception. But getFull() doesn't handle it.

I'm a little fuzzy on error handling/thowing in promises. I guess it's a case of wrapping a try block around and calling reject() if an error is thrown. But that seems awfully laborious... And I'll bet it's a reasonably common occurrence throughout the extension API, so maybe there's some way to handle such cases more generally?

getFull():
https://searchfox.org/comm-central/source/mail/components/extensions/parent/ext-messages.js#212

MsgHdrToMimeMessage():
https://searchfox.org/comm-central/source/mailnews/db/gloda/modules/MimeMessage.jsm#247

Blocks: 1644027
No longer depends on: 1644027

Actually MsgHdrToMimeMessage is not throwing, it's just passing null to the callback which isn't helpful. We can take that as a sign of failure and reject.

This brings the result of MsgHdrToMimeMessage out to the main function and throws if it's null.
It should prevent convertMessagePart failing but if it does the failure will at least be returned to the extension.

Assignee: nobody → geoff
Status: NEW → ASSIGNED

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/4181feb3184d
Handle null result from MsgHdrToMimeMessage in messages API. r=john.bieling

Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED

For anybody wondering, John is changing getRaw in bug 1696884 so I didn't touch it.

Target Milestone: --- → 88 Branch

The fix is neat, however since this bug is closed, should not "getFull" be removed from the title? The same question is to "Blocks" relation to the Bug #1696884.

Summary: messages.getFull and getRaw don't reject upon failure → messages.getFull don't reject upon failure

78 uplift?

Flags: needinfo?(john)

Yes, I will prepare a patch.

Comment on attachment 9218950 [details] [diff] [review]
bug1697075_Handle_null_result_from_MsgHdrToMimeMessage_in_messages_API_ESR78.patch

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
unexpected API fails

Testing completed (on c-c, etc.):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2614ffa5c34e6223d22b4935467d9602f2ec0105

Risk to taking this patch (and alternatives if risky):
Low

Flags: needinfo?(john)
Attachment #9218950 - Flags: approval-comm-esr78?

Comment on attachment 9218950 [details] [diff] [review]
bug1697075_Handle_null_result_from_MsgHdrToMimeMessage_in_messages_API_ESR78.patch

[Triage Comment]
Approved for esr78

Attachment #9218950 - Flags: approval-comm-esr78? → approval-comm-esr78+
You need to log in before you can comment on or make changes to this bug.