User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

1. Get a MessageHeader via browser.messageDisplay.onMessageDisplayed for news message inside a nntp account.
2. I tried to get the content by calling message.getFull(id) or message.getRaw(id)

Actual results:

The promises returned by getFull() and getRaw() never get out of the pending state.

Expected results:

The promises should get resolved with the message content. Or at least get rejected with an error.

Confirmed over 78.4.0 as well. In some cases, I get "TypeError: aPart is null MimeMessage.jsm:134:7" for getFull.
In addition, the MessageHeader is also not fully populated, lacking receivers, CC lists, possibly more.

Any chance that this get fixed? Makes the WebExtension API unusable for my add-on, enforcing own API extensions again.

nntp://news.gmane.io is remaining an extremely convenient interface
to mail lists even though HTTP interface of gmane.org was shut down
long time ago. It is possible to follow mail lists without
subscription. Archives are available, so there is no problem
to access from mail client messages sent before subscription to
a mail list.

My hope was that I could easily write an extension that extracts
useful headers from messages for my notes. Reality is that
only a few of headers are directly available. Even to get "To"
or "newsgroups" headers, it is necessary to query for a full message,
not to say about links to http archive or gmane message id.
I do not mind to have "Date" in the timezone of the sender.
Even "Message-Id" is available in nightly but not in the thunderbird-68.10
shipped with Ubuntu-20.04 focal (current long term support release).
The final disappointment is that messages.getFull(id) does not work at all
and messages.getRaw(id) is rather unstable.
I was surprised that attempt get details of message that is displayed
on the screen could fail due to timeout. It is a pity that the only
reliable way is to open message source and to cherry pick interesting
headers using mouse.

Steps to reproduce.

• Setup news account for news.gmane.io
• subscribe to some group, e.g. gmane.linux.debian.user.announce.
  To factor out enigmail/OpenPGP issues, some other group could be used, e.g.
  gmane.comp.mozilla.thunderbird.user
• Load temporary extension
• invoke extension action "getRaw" through the context menu for message list.
  At first, let's try to do it without displaying the message
  using left click on it.
• inspect extension and thunderbird console messages.

Test extension manifest.json

{
	"manifest_version": 2,
	"name": "getFull test",
	"version": "0.1",
	"permissions": [
		"messagesRead", "menus"
	],
	"background": {
		"scripts": [ "background.js" ]
	}
}

background.js

async function testActionHandler(info, tab) {
	let selectedMessages = info && info.selectedMessages
		&& info.selectedMessages.messages;
	for (const message of selectedMessages) {
		if (info.menuItemId === "TEST_GET_FULL") {
			console.log("calling getFull() for message %o", message);
			const full = await browser.messages.getFull(message.id);
			console.log(full);
		} else if (info.menuItemId === "TEST_GET_RAW") {
			console.log("calling getRaw() for message %o", message);
			const raw = await browser.messages.getRaw(message.id);
			console.log(raw.substring(0, 1024));
		} else {
			console.error("testActionHandler: unsupported action %o", info.menuItemId);
		}
	}
}
function testMain() {
	browser.runtime.onInstalled.addListener(function testCreateMenu() {
		browser.menus.create({
			contexts: [ "message_list", "selection", "link", "page", "frame" ],
			id: "TEST_GET_FULL",
			title: "getFull test",
		});
		browser.menus.create({
			contexts: [ "message_list", "selection", "link", "page", "frame" ],
			id: "TEST_GET_RAW",
			title: "getRaw test",
		});
	});

	browser.menus.onClicked.addListener(function testMenuListener(info, tab) {
		console.debug("testMenuListener", info, tab);
		testActionHandler(info, tab).catch(ex => console.error("testMenuListener:", ex));
	});
}

testMain();

Extension console (thunderbird 87.0a1 2021-02-21)

testMenuListener 
Object { menuItemId: "TEST_GET_FULL", parentMenuItemId: 0, viewType: "tab", editable: false, pageUrl: undefined, selectedMessages: {…}, modifiers: [], button: 0 }
 
Object { id: 1, index: 0, windowId: 3, highlighted: true, active: true, status: "complete", width: 823, height: 664, mailTab: true }
background.js:32:11
calling getFull() for message  
Object { id: 1, date: Date Thu Jul 09 2020 16:24:11 GMT+0000 (Coordinated Universal Time), author:  … }
background.js:6:11
testMenuListener: Error: An unexpected error occurred background.js:33:52
    testMenuListener moz-extension://010f5daf-3f51-463e-9b7b-1818baa2cbc9/background.js:33
    (Async: promise callback)
    testMenuListener moz-extension://010f5daf-3f51-463e-9b7b-1818baa2cbc9/background.js:33

Thunderbird console

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgMessageService.streamMessage]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource:///modules/gloda/MimeMessage.jsm :: MsgHdrToMimeMessage :: line 236"  data: no] MimeMessage.jsm:236:16
    MsgHdrToMimeMessage resource:///modules/gloda/MimeMessage.jsm:236
    getFull chrome://messenger/content/parent/ext-messages.js:212
    getFull chrome://messenger/content/parent/ext-messages.js:210
    getFull self-hosted:1173
    result resource://gre/modules/ExtensionParent.jsm:956
    withPendingBrowser resource://gre/modules/ExtensionParent.jsm:489
    result resource://gre/modules/ExtensionParent.jsm:956
    callAndLog resource://gre/modules/ExtensionParent.jsm:918
    recvAPICall resource://gre/modules/ExtensionParent.jsm:955
    AsyncFunctionNext self-hosted:690

Does not work, but at least the promise has been rejected.

• Now try to display some message by left click on the entry in the message list
• Invoke "getRaw" context menu for the same item.

Silent failure, the promise is pending forever. Another error in logged
to thunderbird console:

Did you try to stream an nttp message? Doens't work. See bug 545365. MimeMessage.jsm:121:13
    onDataAvailable resource:///modules/gloda/MimeMessage.jsm:121
TypeError: right-hand side of 'in' should be an object, got nullext-messages.js:54:9
    convertMessagePart chrome://messenger/content/parent/ext-messages.js:54
    getFull chrome://messenger/content/parent/ext-messages.js:216
    onStopRequest resource:///modules/gloda/MimeMessage.jsm:95

Try extension "getRaw" action twice for the same message.
For the first time it works, next attempts leads to silent failures.

My expectations:

1. messages.getFull and messages.getRaw works for news (NNTP) messages.
2. no hanged promises, any failures cause promise rejection.
3. Error messages are informative enough to figure out what has happened,
   not just something like "unexpected error".

P.S. In my particular use case, I do not need message body or attachments,
it is enough to fetch just message headers.

As to the following error

TypeError: right-hand side of 'in' should be an object, got nullext-messages.js:54:9
    convertMessagePart chrome://messenger/content/parent/ext-messages.js:54
    getFull chrome://messenger/content/parent/ext-messages.js:216
    onStopRequest resource:///modules/gloda/MimeMessage.jsm:95

that appears in thunderbird (not extension) console, it is due to insufficient error handling in convertMessagePart https://hg.mozilla.org/comm-central/file/tip/mail/components/extensions/parent/ext-messages.js#l54 To get such message, try "getFull" (e.g. using test extension from the previous comment) for a NNTP message that is displayed or for a NNTP message for that "getRaw" was called earlier. The function is not ready that its argument could be null. I suppose, it should just return in this case.

There is more severe problem with getFull implementation https://hg.mozilla.org/comm-central/file/tip/mail/components/extensions/parent/ext-messages.js#l216 It is not ready that convertMessagePart() could throw an exception. Such exception must cause rejection of the promise. If mimeMsg at this point is null than it could be considered as a reason to reject the promise per se. I am unsure if try-catch around msgService.streamMessage is enough in https://hg.mozilla.org/comm-central/file/tip/mailnews/db/gloda/modules/MimeMessage.jsm#l236 or return value should be checked as well.

I believe, neither fulfilled nor rejected promise is a serious error.

I have filed the Bug #1694988 "nsNntpService::StreamMessage does not support aConvertData argument". It is the actual origin of the errors described above. Without a fix of #1694988, messages.getFull() would not work. The only hope is to get at least an error reliably.

In my opinion, unconditionally passing null as the aAllowDownload argument of MsgHdrToMimeMessage is questionable, see https://hg.mozilla.org/comm-central/file/tip/mail/components/extensions/parent/ext-messages.js#l218 If an NNTP message has not been retrieved before (message neither has been displayed nor e.g. message.getRaw() has been called), then the following error appears in thunderbird console

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgMessageService.streamMessage]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource:///modules/gloda/MimeMessage.jsm :: MsgHdrToMimeMessage :: line 236"  data: no] MimeMessage.jsm:236:16
    MsgHdrToMimeMessage resource:///modules/gloda/MimeMessage.jsm:236
    getFull chrome://messenger/content/parent/ext-messages.js:212
    getFull chrome://messenger/content/parent/ext-messages.js:210
    getFull self-hosted:1173
    result resource://gre/modules/ExtensionParent.jsm:956
    withPendingBrowser resource://gre/modules/ExtensionParent.jsm:489
    result resource://gre/modules/ExtensionParent.jsm:956
    callAndLog resource://gre/modules/ExtensionParent.jsm:918
    recvAPICall resource://gre/modules/ExtensionParent.jsm:955
    InterpretGeneratorResume self-hosted:1480
    AsyncFunctionNext self-hosted:690

Promise returned to an extension is rejected but with obscure error "Error: An unexpected error occurred"

I have split up this bug. The news MessageHeader does not include recipients or ccList because of bug 1696895. Either that has to be fixed or we have to manually poll those directly from the headers in the API.

I think the fact the error is being suppressed is also a separate issue - I've just written it up in Bug 1697075.

So getFull requires fix of nsNntpService.

Notice that the fix applied for getFull in the Bug #1697075 does not address the issue from comment #4: either aAllowDownload argument of MsgHdrToMimeMessage should be changed to true or additional options argument should be added to getFull to allow extension developers to control if the message could be downloaded from remote server.

For getRaw a workaround could be implemented for older thunderbird versions using experiment API, see a comment to Bug #1696884 (extensions). The problem appeared due to a difference of nsNntpService behaviour in comparison to other message service implementations. It is addressed by the Bug #1695235. Either fix of the Bug #1695235 or Bug #1696884 is enough to make getRaw working.

We cannot use MsgHdrToMimeMessage for nsNntpService as that does not support the emitter=js and/or aConvertData (MimeMessageEmitter.initialize() is not invoked by MsgHdrToMimeMessage for nntp). I am currently trying to find a way to get the headers and body without using MsgHdrToMimeMessage, but that is not as easy as it sounds.

The headers will be doable, so it looks like the API's MessageHeader can be fixed for nntp, but I am not so sure about the mime parts. Still learning.

In 1623685 Comment 11 I asked for STR to get partially downloaded messages to play with the aAllowDownload argument.

(In reply to John Bieling (:TbSync) from comment #8)

We cannot use MsgHdrToMimeMessage for nsNntpService as that does not support the emitter=js and/or aConvertData (MimeMessageEmitter.initialize() is not invoked by MsgHdrToMimeMessage for nntp). I am currently trying to find a way to get the headers and body without using MsgHdrToMimeMessage, but that is not as easy as it sounds.

I wrote it assuming that at some moment nsNntpService::StreamMessage could get support of aConvertData and extensions could be prepared.

I have described a workaround how to get parsed headers from NNTP message raw text in the Bug #1696895.

In Bug #1623685 Comment 11 I asked for STR to get partially downloaded messages to play with the aAllowDownload argument.

The issue is the same, I have added a comment there.

@max: aAllowDownload has been switched to true by another patch (the one adding the listAttachments method):
https://phabricator.services.mozilla.com/D107403

IIRC, the last remaining issue pointed out in here is getFull. I will go for jsmime and mimic the return value of the old mime parser, partially stealing code from the openPGP implementation, which is already doing its own thing independent of the cpp nntp service implementation.

I have a worry about Promise.all in https://phabricator.services.mozilla.com/D107825 (Bug #1697481). It seems that whole bunch could be rejected due to a single network error.

The reason is not related to extensions however. I have some problem with wifi that is not annoying enough to seriously debug it (and I have no bright idea how to do it). In browser it appears as a pause during loading a page. Usually the problem appears unexpectedly and could gone away without any action (that makes it harder to find its origin). Thunderbird is more sensitive: RSS feeds regularly shows warning sign. The most severe consequence that I experienced a few times is blank message pane for a news message. It seems that fetched state is cached somewhere but actually the message is not obtained due to network error. Once it happened when I turned off and on wifi.

I am not against the patch, I just warn that in some circumstances it could lead to less stable behaviour. I could not suggest a better approach.

I need to use promises there, as convertMessage had to be made async. Is promise.all rejecting completely, as soon as one of those promises rejects? What we want is to wait for all, grab all the data we can, but skip those who failed...

I could catch the error directly in convertMessage, and return nothing for ccList/recipients (or some error indication) and let the final promise which is going into Promise.all() always succeed, could I not?

I am in doubt if bccList field should be added to the workaround. Composing a message, I could add such header, but it will be available in sent mail only (that is not news message), the same message appeared in a news group will not have it.

(In reply to John Bieling (:TbSync) from comment #12)

I need to use promises there, as convertMessage had to be made async.

I think, the best way is to fix nsNntpService or at least to raise priority of the related bug.

Is promise.all rejecting completely, as soon as one of those promises rejects?

With Promise.all method any error cause immediate rejection. Promise.allSettled is a relatively recent addition. Personally, within extension I decided to wrap methods that could cause slow requests into Promise.race with timeout. As to extension API, maybe abort controller (similar to existing in fetch API) could be passed as an option. However I do not have experience with abort controllers in practice.

What we want is to wait for all, grab all the data we can, but skip those who failed...

Variant with Promise.all could cause significant delay in the case of unstable network connection and rather high probability to get an error, so extension developers will have to add timeouts and retries. Adding values only for messages with successfully obtained raw messages results in ambiguity if the value is really missed or just a request has failed. I could not imaging how to mark some MessageHeader instances as partially filled in current API.

The arguments are no more than my opinion, feel free to ignore them.

I think, list operations should be cheap and fast and I consider acceptable if it provides no more information than corresponding command of the underlying protocol. Unfortunately I am not familiar with details of NNTP protocol, so unsure if "Cc" and "To" are included into server response when list of messages is obtained (or "Newsgroups" is an equivalent for recipients in the NNTP world). So in extension API, list should return already available information and should not cause bunch of network requests. There should be a kind of backpressure, developer should feel that some operation is expensive. Helpers for widely used operations could exist but basic functions should be reliable and fast when possible. If some protocols impose limitations what headers are available, it should be just documented.

So I am against of fixing the recipients/Cc bug in the code providing extensions API due to possible request avalanche. However it is up to you to decide if such approach is acceptable.

With Promise.all method any error cause immediate rejection. Promise.allSettled is a relatively recent addition.

I actually did not know that, glad I got pointed to the documentation. I decided against using Promise.allSettled as it has a slightly different return value which would require additional code changes, which I did not want to do. So I caused the Promise which is returned by convertMessage() to always resolve.

I could not imaging how to mark some MessageHeader instances as partially filled in current API.

How about, setting them to false?

I think, list operations should be cheap and fast and I consider acceptable if it provides no more information than corresponding command of the underlying protocol.
So I am against of fixing the recipients/Cc bug in the code providing extensions API due to possible request avalanche. However it is up to you to decide if such approach is acceptable.

So you are saying, we should not implement this workaround and have this unfixed until bug 1696895 is fixed? This is beyond what I can do, I guess. Or should we think about a populate option, which enables this extra code path on demand?

Alternatively, I was thinking about adding a getHeaders() method to the API, which only gets the headers (which will work on imap and news). This would allow the developer to still get to and cc and whatnot, and we would document that news does not include those in the cheap MessageHeader.

So I caused the Promise which is returned by convertMessage() to always resolve.

It is quite reasonable, however slow network connection or server response could result in significant delay.

I could not imagine how to mark some MessageHeader instances as partially filled in current API.

How about, setting them to false?

Maybe, but it would be surprise for some code to get Boolean instead of Array...

So you are saying, we should not implement this workaround and have this unfixed until bug 1696895 is fixed? This is beyond what I can do, I guess. Or should we think about a populate option, which enables this extra code path on demand?

Yes, I am. I believe that possible burst of network requests is too high overhead for a method that should be cheap and fast by design. If possible, such bug should be fixed in proper place. If it is unfeasible due to underlying protocol, it should be clearly documented. For list operation I would rather prefer separate functions, not optional arguments to existing one to increase probability that extension developers will realize that such function could be expensive in the sense of network request count, slow, unreliable. Higher level API could hide complexity, but it should provide some notion if operation is expensive.

As candidate for optional argument I have suggested a controller that could cancel requests that has not completed. For methods operating with single message (implemented with 1 or 2 underlying network requests) a kind of aLocalOnly option is reasonable in my opinion.

Alternatively, I was thinking about adding a getHeaders() method to the API, which only gets the headers (which will work on imap and news). This would allow the developer to still get to and cc and whatnot, and we would document that news does not include those in the cheap MessageHeader.

It would be great to have getHeaders() since anyway I have implemented a kind of polyfill for such function for my extension. My personal concern is raw values in addition to structured ones. I would like to have "Date" in the timezone of sender. However I am unaware to which degree some headers could be "potentially expensive" for structuring in reality:
https://hg.mozilla.org/comm-central/file/tip/mailnews/mime/jsmime/jsmime.js#l1717

If you have potential code which we could use for getHeaders - based on the raw msg returned by MsgHdrToRawMessage - feel free to open a bug for that. Any help is appreciated.

I will not add additional elements like an abort controller, as that seems to make the API overcomplicated. I have posted to discuss.thunderbird.net and based on that feedback I will back this out completely or add an optional flag to do the additional network request in case it is a single get request.

I do not think, my code has any real value. I have written it partially to probe if I would face some other problems or limitations. To get raw message, my implementation is almost similar to MsgHdrToRawMessage, I even do not discard remaining part of a message as soon as empty line is received (it could happen on chunk borders). Neither I have tried to avoid potentially expensive structuring (e.g. by passing a list of interesting headers). Currently I just serialize map-like objects to arrays to pass them through extension border:

const headers = MimeParser.extractHeaders(rawText);
retval.headers = Array.from(headers);
retval.rawHeaders = Array.from(headers.keys(), h => [h, headers.getRawHeader(h)]);

So I have mail addresses split from mail names (I have not checked group addresses yet) and raw date value with original time zone.

The attached patch is a very preliminary concept using jsmime to get a MessagePart object for news. In the tree we have two implementations using jsmime.

MimeParser: https://searchfox.org/comm-central/source/mailnews/mime/src/mimeParser.jsm
EnigmailMime: https://searchfox.org/comm-central/source/mail/extensions/openpgp/content/modules/mime.jsm

The first one has no emitter which actually returns the mime subparts, so I use the second one. There are a few differences to the output generated by MsgHdrToMimeMessage:

• size is not supported
• Headers differ slightly, MsgHdrToMimeMessage strips \t while Enigmail.getMimeTree does not. Also, getMimeTree includes sequences like =?us-ascii?Q?1;DB5PR0201MB1463;23:Eg55ihvY4g752nWbP0chgz1qKWYuSIds73YLiq8?=.

Ideas on how to fix those?

We currently do not have to worry about encryption, as news are most probably not encrypted, but in the very end we should have a full-fledged jsmime based MimeParser, which also supports S/MIME and PGP. So all the different implementations we have should be merged. For now, I will probably copy the missing emitter pieces of EnigmailMime into MimeParser (including attachment support) and use that.

Is there any benefit, to no longer get the raw mail first and parse the string, but use the async streamer support of MimeParser? Can that skip attachments (which is the only benefit I can see)?

What do you think?

Updated patch has the MimeParser implementation updated to return a MsgHdrToMimeMessage compatible object for full mime parsing of emails (so it could be used as a replacement for MsgHdrToMimeMessage ), but I was not yet able to get the attachments right. And no encryption support.

Attachments work now as well.

Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/7471ec244f7d
Fix messages.getFull() and attachment support for news by adding a full MIME emitter to jsmime.js based MimeParser.jsm. r=mkmelin

Sorry for the late response.

getMimeTree includes sequences like =?us-ascii?Q?1;DB5PR0201MB1463;23:Eg55ihvY4g752nWbP0chgz1qKWYuSIds73YLiq8?=

I tried to add similar changes through experiments API (I am unsure whether nightly builds contained the commit). Headers are decoded but message body (message parts) looks like trash in the case of non-ASCII encoded as UTF-8 + base64 or quoted printable. However it is possible that I did something wrong.

I am unsure whether nightly builds contained the commit

The commit had to be backed out, due to test fails on linux/mac. The updated patch works on all plattforms, but needs to be reviewed again.

getMimeTree includes sequences like =?us-ascii?Q?1;DB5PR0201MB1463;23:Eg55ihvY4g752nWbP0chgz1qKWYuSIds73YLiq8?=

From my own tests, the patch is able to cope with such sequences now.

Patch has been updated and try run is looking good:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=eed3c73cc0e88719bc74410c90ed3091ae6dfb89

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/540c67e3c82d
Fix messages.getFull() and attachment support for news by adding a full MIME emitter to jsmime.js based MimeParser.jsm. r=mkmelin

(In reply to max from comment #25)

message body (message parts) looks like trash in the case of non-ASCII encoded as UTF-8 + base64 or quoted printable.

90.0a1 (2021-05-27) (64-bit)

I have no complains concerning headers.

Message body is still rather garbage while the message is in its original news folder:
"\r\nÐ\u0095Ñ\u0081ли Ñ\u0081пеÑ\u0080ва запÑ\u0083Ñ\u0081Ñ\u0082иÑ\u0082Ñ\u008c"
If I copy such message to local folders then getFull is able to properly decode message parts.

Could you create a new bug with detailed information?
Is that a public news server I can subscribe to? I do not know if I can reproduce this with the message source, but that would be helpful as well.
Is getFull(), getRaw() or both failing this?

Do you have already an idea what is causing this?

See Bug #1713145 that demonstrates that namely MimeParser.extractMimeMsg is unable to decode UTF-8 body transferred as base64. Quoted printable has the same problem. It is applicable to multipart messages as well.

messages.getRaw does not have such problem since it should return encoded body.

I have no idea at which stage message body is decoded. Some time ago I spent some time trying to figure out how to decode headers, but I stopped as soon as I found an acceptable workaround (MimeParser.extractHeaders(rawText)).