Open Bug 1894672 Opened 7 months ago Updated 4 days ago

Intermittent comm/mail/base/test/browser/browser_getMessages_certError.js | showAlert should not be called while an alert is showing

Categories

(Thunderbird :: General, defect, P5)

Product:
Thunderbird
Component:
General
Type:
defect

Tracking

(Not tracked)

Status:
REOPENED
Milestone:
129 Branch

People

(Reporter: intermittent-bug-filer, Unassigned)

References

(Regression)

Details

(Keywords: intermittent-failure, intermittent-testcase, regression)

Attachments

(1 file)

Bug 1894672 - Wait for repeated alerts before moving on in connection error tests. r=#thunderbird-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Keywords: regression
Regressed by: 1893899
Summary: Intermittent comm/mail/base/test/browser/browser_getMessages_certError.js | single tracking bug → Intermittent comm/mail/base/test/browser/browser_getMessages_certError.js | showAlert should not be called while an alert is showing
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Duplicate of this bug: 1894772
Keywords: checkin-needed-tb
Target Milestone: --- → 129 Branch

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/2543f8548643
Wait for repeated alerts before moving on in connection error tests. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED

Still happening :(

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

I think there must be an actual bug (or more than one) here. It seems all the failures are from the same point in the test now. At that point the code shouldn't be making two alerts in the first place, and second one is within the 1000ms waiting period.

Unfortunately whenever I try to reproduce this on the try server it refuses to happen.

I wonder if it's the first alert that's not supposed to happen. That could happen as the IMAP code might make more than one alert (the POP3 code shouldn't but that's where the failure occurs) and they'd have the same text.

I should review the changes that were made in bug 1893899.

Flags: needinfo?(kaie)

I'm testing this on Fedora Linux with the Gnome Desktop.

When I get a bad cert error, I get an error popup.
I don't know how that type of popup is called, it's shown outside of Thunderbird, it's shown in the desktop environment's area with all notifications.

Is that the type of alert that's mentioned here?

It seems that alert isn't modal.

If it isn't modal, why can be a problem to trigger another alert, while the first one is still showing?

Because it's pointless to show several identical notifications just because the code is coded badly. That's what is happening, there's not separate problems for the user to know about.

This test failure is (I think) due to the CI machines being resource constrained and things taking longer than expected. We should wait for an event instead of an arbitrary time, but that's very difficult in this case.

It isn't clear to me which of the following happens, can you clarify?

(a) We have logic to detect additional notifications, and that code should prevent the additional ones, but that code isn't working.

(b) We don't expect being notified about a cert error twice, but that's what happens, and that triggers the duplicate notification?

The test has this comment:
// There could be multiple alerts for the same problem. These are swallowed
// while the first alert is open, but we should wait a while for them.

I cannot find the code that is expected to swallow (suppress) alerts while the first one is open.

(In reply to Kai Engert (:KaiE:) from comment #18)

I cannot find the code that is expected to swallow (suppress) alerts while the first one is open.

I think I found it, alertHook.sys.mjs, activeAlerts.

I have a theory:

  • you trigger get messages
  • the cert is triggered and showAlert is called
  • then you call observe("alertclickcallback")
  • alertHook.observe opens an exceptionDialog, with prefetchCert: true,
    might that trigger the subsequent network activity and the new cert error?
  • immediately after you request the dialog, you call activeAlerts.delete(),
    so from now on, the same cert error is no longer suppressed,
    which means, if the dialog triggers the error, onCertError will call showAlert
  • however, at this time, MockAlertsService._alert is still set
Flags: needinfo?(kaie)

Hmm, that exception dialog is modal. If the call to openDialog blocks until after the dialog gets closed, then activeAlerts still has the tracker entry and should cause suppression...

I was able to reproduce locally, it happened when I had lots of logging enabled.

we start the notyetvalid test
we get the IMAP cert error
we run through the exception dialog, the exception is present
we reach alertfinished
I see again "Requesting notyetvalid.test.test:993"
again we arrive in onCertError for IMAP :993
the alert is shown

However, before we reach observe("alertclickcallback"),
we reach onCertError for POP :995,
which also calls showAlert

So we are in fact showing different errors in parallel, not the same one, at least in the scenario that I see.

Instead of one global MockAlertsService, would it work to create a separate instance of MockAlertsService for each port/protocol you're testing, and have a member variable instead of a static member?

(In reply to Kai Engert (:KaiE:) from comment #22)

Instead of one global MockAlertsService, would it work to create a separate instance of MockAlertsService for each port/protocol you're testing, and have a member variable instead of a static member?

Maybe that doesn't work, if only one global mock object may be registered at any time.

Assignee: geoff → nobody
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: