Closed Bug 1695235 Opened 3 years ago Closed 9 months ago

Calls of aUrlListener from nsNntpService::StreamMessage are unreliable

Categories

(MailNews Core :: Networking: NNTP, defect)

defect

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: manikulin, Unassigned)

References

Details

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

I was trying to write an extension that extracts some information from message headers for my notes. At first, I faced that messages.getFull(id) does not for news (NNTP) messages at all. For a next attempt I tried messages.getRaw(id). Sometimes it works but it hardly could be considered as reliable.

Finally I have realized that the problem is that methods of the aUrlListener argument of streamMessage function are invoked only if the message is fetched for the first time but not for next attempts. It causes the Bug #1644027, more precisely its part related to getRaw. I suspect that getFull part of that issue is caused by the Bug #1694988.

To reproduce, consider the following test extension:

Essentially the code is taken from https://hg.mozilla.org/comm-central/file/6a07f13de6c867f0a8650e47733f8f46872d6ffb/mail/components/extensions/parent/ext-messages.js#l223 with additional logging and removed promise.

experiment.js

"use strict";
var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
ChromeUtils.defineModuleGetter(
  this,
  "NetUtil",
  "resource://gre/modules/NetUtil.jsm"
);

var tst = class extends ExtensionCommon.ExtensionAPI {
	getAPI(context) {
		return { tst: {
			async getRaw(messageId) {
				let messenger = Cc["@mozilla.org/messenger;1"].createInstance(
					Ci.nsIMessenger
				);
				const msgHdr = context.extension.messageManager.get(messageId);
				let msgUri = msgHdr.folder.generateMessageURI(msgHdr.messageKey);
				let service = messenger.messageServiceFromURI(msgUri);

				let streamListener = Cc[
					"@mozilla.org/network/sync-stream-listener;1"
				].createInstance(Ci.nsISyncStreamListener);
				const uri = service.streamMessage(
					msgUri,
					streamListener,
					null,
					{
						OnStartRunningUrl(url) { console.log("OnStartRunningUrl", url); },
						OnStopRunningUrl(url, exitCode) {
							console.log("OnStopRunningUrl", url, exitCode);
							if (exitCode !== 0) {
								Cu.reportError(exitCode);
							} else {
								const raw = NetUtil.readInputStreamToString(
									streamListener.inputStream,
									streamListener.available()
								);
								console.log(raw && raw.length, raw && raw.substring(0, 256));
							}
						},
					},
					false,
					""
				);
			},
		}, };
	}
	onShutdown(isAppShutdown) {
		if (!isAppShutdown) {
			Services.obs.notifyObservers(null, "startupcache-invalidate", null);
		}
	}
};

background.js

"use strict";
function testActionHandler(info, tab) {
	let selectedMessages = info && info.selectedMessages
		&& info.selectedMessages.messages;
	for (const message of selectedMessages) {
		if (info.menuItemId === "TEST_GET_RAW") {
			console.log("calling getRaw() for message %o", message);
			browser.tst.getRaw(message.id);
		} else {
			console.error("testActionHandler: unsupported action %o", info.menuItemId);
		}
	}
}
function testMain() {
	browser.runtime.onInstalled.addListener(function testCreateMenu() {
		browser.menus.create({
			contexts: [ "message_list" ],
			id: "TEST_GET_RAW",
			title: "getRaw test",
		});
	});

	browser.menus.onClicked.addListener(function testMenuListener(info, tab) {
		console.debug("testMenuListener", info, tab);
		testActionHandler(info, tab);
	});
}
testMain();

schema.json

[ {
	"namespace": "tst",
	"functions": [ {
		"name": "getRaw",
		"type": "function",
		"parameters": [ { "name": "messageId", "type": "integer" } ]
	} ]
} ]

manifest.json

{
	"manifest_version": 2,
	"name": "getRaw test",
	"version": "0.1",
	"permissions": [ "messagesRead", "menus" ],
	"background": { "scripts": [ "background.js" ] },
	"experiment_apis": { "tst": {
		"schema": "schema.json",
		"parent": {
			"scopes": [ "addon_parent" ],
			"paths": [ [ "tst" ] ],
			"script": "experiment.js"
		}
	} }
}
  • add news account for news.gmane.io, it is a great archive of mail lists
  • subscribe to some group, e.g. gmane.comp.mozilla.thunderbird.user
  • invoke "getRaw test" context menu for some message in the list
  • inspect extension console and thunderbird console for log message
  • invoke "getRaw test" for the same message more times
  • inspect logs again

Actual results:

  • First time aUrlListener methods are called (thunderbird console)
testMenuListener
calling getRaw() for message
WebExtensions: OnStartRunningUrl
WebExtensions: OnStopRunningUrl
WebExtensions: 6567 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail
  • Next time callbacks are not invoked
testMenuListener
calling getRaw() for message

In a real extension it means that promise is never settled. Neither result nor error is reported to the caller.

Expected results:

  • messages.getRaw() extension API method should not hang, it should return either value or error
  • so nsNntpService::StreamMessage should invoke callbacks of its aUrlListener argument

P.S. I have an impression that code of nsNntpService::StreamMessage and nsNntpService::DisplayMessage` are so similar that both methods could be implemented by calling a generalized method to avoid code duplication. However I have no idea which variant is more reliable.

Summary: Calls of aUrlListener from nsNntpService::StreamMessage is unreliable → Calls of aUrlListener from nsNntpService::StreamMessage are unreliable

Maybe Ben wants to give all that streaming stuff a bit of TLC. Looks like you found a few NNTP issues, this one as well as bug 1696449 and bug 1694988.

On the extension side it is about to be fixed in Bug #1696884.

As to code of nsNntpService::StreamMessage and nsNntpService::DisplayMessage, it seems that e.g. conditions, if port number should be set, are a bit different.

See Also: → 1696449

nsNNTP code no longer exists since nntp-js. If the news behavior is still flawed, please file a new bug report.

Status: UNCONFIRMED → RESOLVED
Closed: 9 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.