Add some logging for spurious diff closing issue
Release-Notes: skip
Google-Bug-Id: b/235808180
Change-Id: I6c0275ee415f09cba3a9d99c591a4dc2d077b408
diff --git a/polygerrit-ui/app/constants/reporting.ts b/polygerrit-ui/app/constants/reporting.ts
index 239c6ee..325787f 100644
--- a/polygerrit-ui/app/constants/reporting.ts
+++ b/polygerrit-ui/app/constants/reporting.ts
@@ -128,4 +128,11 @@
COMMENTS_AUTOCLOSE_CHECKS_UPDATED = 'comments-autoclose-checks-updated',
COMMENTS_AUTOCLOSE_THREADS_UPDATED = 'comments-autoclose-threads-updated',
COMMENTS_AUTOCLOSE_COMMENT_REMOVED = 'comments-autoclose-comment-removed',
+ // The following interactions are logged for investigating a spurious bug of
+ // auto-closing diffs.
+ DIFF_AUTOCLOSE_DIFF_UNDEFINED = 'diff-autoclose-diff-undefined',
+ DIFF_AUTOCLOSE_DIFF_ONGOING = 'diff-autoclose-diff-ongoing',
+ DIFF_AUTOCLOSE_RELOAD_ON_WHITESPACE = 'diff-autoclose-reload-on-whitespace',
+ DIFF_AUTOCLOSE_RELOAD_ON_SYNTAX = 'diff-autoclose-reload-on-syntax',
+ DIFF_AUTOCLOSE_RELOAD_FILELIST_PREFS = 'diff-autoclose-reload-filelist-prefs',
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 88255b7..b04c63f 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -60,7 +60,7 @@
import {GrCursorManager} from '../../shared/gr-cursor-manager/gr-cursor-manager';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {ParsedChangeInfo, PatchSetFile} from '../../../types/types';
-import {Timing} from '../../../constants/reporting';
+import {Interaction, Timing} from '../../../constants/reporting';
import {RevisionInfo} from '../../shared/revision-info/revision-info';
import {select} from '../../../utils/observable-util';
import {resolve} from '../../../models/dependency';
@@ -1598,6 +1598,10 @@
if (!this.diffs.length) {
return;
}
+ this.reporting.reportInteraction(
+ Interaction.DIFF_AUTOCLOSE_RELOAD_FILELIST_PREFS
+ );
+
// Re-render all expanded diffs sequentially.
this.renderInOrder(this.expandedFiles, this.diffs);
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 71f596f..b890f21 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -563,6 +563,9 @@
return this.reloadPromise;
}
+ // for DIFF_AUTOCLOSE logging purposes only
+ private reloadOngoing = false;
+
async reloadInternal(shouldReportMetric?: boolean) {
this.reporting.time(Timing.DIFF_TOTAL);
this.reporting.time(Timing.DIFF_LOAD);
@@ -572,6 +575,10 @@
this.diff = undefined;
this.errorMessage = null;
const whitespaceLevel = this.getIgnoreWhitespace();
+ if (this.reloadOngoing) {
+ this.reporting.reportInteraction(Interaction.DIFF_AUTOCLOSE_DIFF_ONGOING);
+ }
+ this.reloadOngoing = true;
try {
// We are carefully orchestrating operations that have to wait for another
@@ -581,6 +588,11 @@
// assets in parallel.
const layerPromise = this.initLayers();
const diff = await this.getDiff();
+ if (diff === undefined) {
+ this.reporting.reportInteraction(
+ Interaction.DIFF_AUTOCLOSE_DIFF_UNDEFINED
+ );
+ }
this.loadedWhitespaceLevel = whitespaceLevel;
this.reportDiff(diff);
@@ -623,6 +635,7 @@
}
} finally {
this.reporting.timeEnd(Timing.DIFF_TOTAL, this.timingDetails());
+ this.reloadOngoing = false;
}
}
@@ -1362,6 +1375,9 @@
preferredWhitespaceLevel !== loadedWhitespaceLevel &&
!noRenderOnPrefsChange
) {
+ this.reporting.reportInteraction(
+ Interaction.DIFF_AUTOCLOSE_RELOAD_ON_WHITESPACE
+ );
return this.reload();
}
}
@@ -1380,6 +1396,9 @@
if (oldPrefs?.syntax_highlighting === prefs.syntax_highlighting) return;
if (!noRenderOnPrefsChange) {
+ this.reporting.reportInteraction(
+ Interaction.DIFF_AUTOCLOSE_RELOAD_ON_SYNTAX
+ );
return this.reload();
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
index bb7ffe0..95ac40f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
@@ -893,6 +893,7 @@
element.patchRange = createPatchRange(1, 2);
reportStub = sinon.stub(element.reporting, 'reportInteraction');
await element.updateComplete;
+ reportStub.reset();
});
test('undefined', () => {