Page MenuHomePhabricator

Gerrit notification email contains "null"
Closed, ResolvedPublicBUG REPORT


What happens?:
I have a email from gerrit with unexpected "null" between the "comment count" and the "Patchset" text:
[<!-- this line looks unexpected added for visibility]

Attention is currently required from: Umherirrender.
DannyS712 has posted comments on this change by Umherirrender. ( )

Change subject: build: Suppress ObjectTypeHintReturn for upcoming sniff change

Patch Set 2: Code-Review+2

(1 comment)

null             <!-- this line looks unexpected

Fix typo in commit message ("newly" -> "new")

There is no null in the gerrit UI -

What should have happened instead?:
No "null" in the notification.

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:
Gerrit 3.3.5

Event Timeline

The message payload can be retrieved from the change metadata git fetch origin refs/changes/84/710084/meta

commit f79c7956faef859fa07144f50dd1188158f558d1
Author: Gerrit User 6729 <6729@e9e9afe9-4712-486d-8885-f54b72dd1951>
Date:   Thu Aug 5 23:02:11 2021 +0000

    Update patch set 2
    Patch Set 2: Code-Review+2
    (1 comment)
    Patch-set: 2
    Reviewer: Gerrit User 6729 <6729@e9e9afe9-4712-486d-8885-f54b72dd1951>
    Label: Code-Review=+2
    Attention: {"person_ident":"Gerrit User 161 \u003c161@e9e9afe9-4712-486d-8885-f54b72dd1951\u003e","operation":"ADD","reason":"DannyS712 replied on the change"}

diff --git a/baf4d01efa04359553f0b150a1a7b63b80b4bff1 b/baf4d01efa04359553f0b150a1a7b63b80b4bff1
new file mode 100644
index 00000000000..d1b6df84c9b
--- /dev/null
+++ b/baf4d01efa04359553f0b150a1a7b63b80b4bff1
@@ -0,0 +1,21 @@
+  "comments": [
+    {
+      "unresolved": false,
+      "key": {
+        "uuid": "aa2802a5_5f0e0b54",
+        "filename": "/PATCHSET_LEVEL",
+        "patchSetId": 2
+      },
+      "lineNbr": 0,
+      "author": {
+        "id": 6729
+      },
+      "writtenOn": "2021-08-05T23:02:11Z",
+      "side": 1,
+      "message": "Fix typo in commit message (\"newly\" -\u003e \"new\")",
+      "revId": "baf4d01efa04359553f0b150a1a7b63b80b4bff1",
+      "serverId": "e9e9afe9-4712-486d-8885-f54b72dd1951"
+    }
+  ]
\ No newline at end of file

I have looked at a recent email which comes with both plain text and HTML. The plain text payload indeed shows the null text but the HTML ones does not.


gerrit_html.png (310×749 px, 34 KB)

Plain text

gerrit_text.png (337×790 px, 37 KB)

The email comes from templates and I found out that we forked the upstream ones for some reason, the sources are in operations/puppet:

$ git ls-files modules/gerrit/ |grep soy

Our has:

{for $group in $commentFiles}
  // Insert a space before the newline so that Gmail does not mistakenly link
  // the following line with the file link. See issue 9201.

Which look related to and is an aftermath of the Gerrit 3.3 upgrade. There are two changes upstream to the file:

The variable is removed and I think that is the one resulting in a showing up a null.

Looks like the template got introduced in 2017 by 84c651bfc6e8db67933e6322f37efa082946d259 for T43608. I think I will drop them in favor of using the upstream built in templates.

Change 768005 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/puppet@production] gerrit: prevent 'null' entry in email

Change 768005 merged by Alexandros Kosiaris:

[operations/puppet@production] gerrit: prevent 'null' entry in email

Mentioned in SAL (#wikimedia-releng) [2022-03-09T17:08:08Z] <hashar> Updating Gerrit to get rid of a literal null string being inserted in notification emails | |

This should be solved now