Redownload of failed MMS on delete

[jezek]

• On failed MMS download (or some other message handling error), mNotificationInd is saved and telepathy-ofono is informed with an error message with details in json format.
• Added mNotificationInd Received field and add Recieved time to tepepathy-ofono message
• The mNotificatoinInd Expiry field handles absolute/relative token upon decoding now and the Expiry field is date-time from now on.
• On MMS deletion request, if the message state in storage indicates, that the message is not downloaded yet (or other error occurred), the message is not deleted from storage and keeps listening to further redownload requests.
• On MMS redownload request the message stops listening to requests, is remved and the saved mNotificationInd is handled again, with a indication to remove the previous error message from history.
• If more notifications with the same transaction id occur with download failure (or other error), the error message is communicated only the first time (fixes multiple error messages for some operators).
• On modem identification (upon nuntium initialization), messages in storage are tried to recover if they were some errors and telepathy-ofono is notified to spawn handlers for them. Also old incoming, expired and deleted/read message files are removed from storage.
• The nuntium-inject-push command gets a new & edited flags:
  • --denial-count N (short -dN) - denies download of payload N times.
  • --sender (short -s) - added logic to also affect the payload.
  • --sender-notification - changes sender for the notification only.
  • --transaction-id - adds/changes transaction id to/for notification/payload.
  • --end-point - if empty, it tries to (auto)detect end-point from nuntium logs
  • --error-... - set of flags to simulate various errors (see --help)
• [Architecture documentation] was updated.

Note: This PR is accompanied by ubports/telepathy-ofono#20, gitlab/ubports/core/history-service#8, ubports/telephony-service#20 and ubports/messaging-app#260

Note: Some more reading on forum: https://forums.ubports.com/topic/5100/the-mms-lost-story/39

Note: The MMS deletion request mentioned above comes automatically after mark read request from messaging-app and not after message delete action in the app.

[jezek]

About squashing. There are about 80 commits. I tried to squash them to be less, but It's still about 40 commits. Or I should squash to 1 commit, with the description of this PR inside?

[lduboeuf]

@jezek IMO, we should have 1 commit per new feature or bug fix. But if a feature can be split in several stand-alone parts, we can keep them for easier reading

[jezek]

@lduboeuf The issue is, I did a little bit of changes here and there, sometimes reverted some changes and then tried another approach, while fixing one thing, I fixed some other thing in the same commit. I've tried to squash similar commits together, but it resulted in 40+ commits, as mentioned in previous post. To make 4-5 commits would be as painful as rewriting it again from the start. So the squash to one commit would be the easiest & fastest way. But I will do, as the maintainers say. Who is in charge here? Do I need to ping someone, or are they aware of this?

Note: I know, I made a lot changes, some are unrelated to this issue. To review this must be hell. I apologize and I will try to do anything to ensure smooth review & merging. I've tried to test nearly any error state that could happen during MMS handling and I'm using these PRs as daily-driver without any issues and I think @lduboeuf does too (correct me if I'm wrong).

[lduboeuf]

Maybe @dobey can tell.

Confirmed no issues so far and it works well for me as well.

[gbdomubpkm]

Hello.
Is it possible to simply test this PR as I have done with others more simple or is it too risky/complicated?
If it is possible, please indicate the procedure to follow and the procedure to reverse.
Thank you

[lduboeuf]

@gbdomubpkm
No it is not risky, you can always come back to RC or Stable with the installer if needed
You have to be on devel channel first, then from any terminal run one by one:

 sudo ubports-qa install PR_nuntium_8
 sudo ubports-qa install telepathy-ofono 20
 sudo ubports-qa install history-service 35
 sudo ubports-qa install telephony-service 20
 sudo ubports-qa install messaging-app 260

and reboot

[gbdomubpkm]

@lduboeuf
ok thanks

[NotKit]

@jezek I think having 40+ commits is still better for review than 84. If possible, squash changes that cancel each other if any. I see you added a bunch of new logging messages with "jezek" tag - those probably should be either prettified to look same with rest of debug output or removed.

On unrelated note, I came here trying to debug push parsing issue with Tele2 SE: https://paste.ubuntu.com/p/QhCd57JGsB/. It seems to fail with Download issues: org.freedesktop.DBus.Error.InvalidArgs: Invalid URL: '' on both original and your rewrite, so not directly related, but maybe you have an idea what could be wrong given the scope of changes.

[jezek]

@NotKit

If possible, squash changes that cancel each other if any.

That's what I tried to do and ended up with 40+ commits.

I see you added a bunch of new logging messages with "jezek" tag - those probably should be either prettified to look same with rest of debug output or removed.

All the logging messages with "jezek" tag, are removed in the last commit.

On unrelated note, I came here trying to debug push parsing issue with Tele2 SE: https://paste.ubuntu.com/p/QhCd57JGsB/. It seems to fail with Download issues: org.freedesktop.DBus.Error.InvalidArgs: Invalid URL: '' on both original and your rewrite, so not directly related, but maybe you have an idea what could be wrong given the scope of changes.

The error comes from *MNotificationInd.DownloadContent function called in cmd/nuntium/mediator.go. I did not touch the function. From the "Invalid URL: ''" I can deduce, that the mNotificationInd could not extract url. I did not touch url decoding. Try to make log of the content of mNotificationInd in the DownloadContent function, to see if everything was decoded ok (or in this PR, the mNotificationInd is stored in json format in ~/.local/share/nuntium/store/, so check there). To see, how a valid notification is handled, you can use the nuntium-inject-push tool. Happy hacking.

Note: the log on pastebin shows the "jezek" tag logs, which are all removed in las commit of this PR. Do you use the latest PR version?

[lduboeuf]

Something is wrong with the build version, that explain the issue @NotKit has with "jezek" logs.
If you sudo ubports-qa install PR_nuntium_8 , you will have the 1.4+ubports1+0~20210404194138.35~1.gbp1d6aff even if the last nuild version from this PR is 1.4+ubports1~20210607212135.38~a4fcb8c
Note the "1.4+ubports1+0" vs "1.4+ubports1"
I don't know why, but even the 1.4+ubports1~20210607212135.38~a4fcb8c makes not possible to send or receive MMS

[lduboeuf]

awesome, that was only that ^ ^. I'm surprised thought that debian don't check for Changelog formatting

[jezek]

@lduboeuf you mean it's working now? Thank you for spotting my mistake.

[lduboeuf]

@lduboeuf you mean it's working now? Thank you for spotting my mistake.

Yes, back to a working state :)

[lduboeuf]

@jezek
( from the Post in forum ):

i have a use case where the message is staying in the "pending state" ( so we can't click redownload button anymore ) even after reboot. It is with bad signal and by putting wrong info in MMS settings
In history db the message is x-ubports-nuntium-mms-error-activate-context but the status is still in Pending state

nunitum logs:
2021/06/11 09:15:26 Trying to set Active property to true for context on true /ril_0/context2
2021/06/11 09:15:26 Cannot set Activate to true (try 1/3) interface on /ril_0/context2: org.ofono.Error.NotAttached: GPRS is not attached
2021/06/11 09:15:32 Failed to activate for /ril_0/context2 : failed to activate context
2021/06/11 09:15:32 Cannot activate ofono context: no context available to activate

[lduboeuf]

related issue: ubports/ubuntu-touch#1761

[jezek]

@lduboeuf

i have a use case where the message is staying in the "pending state" ( so we can't click redownload button anymore ) even after reboot. It is with bad signal and by putting wrong info in MMS settings
In history db the message is x-ubports-nuntium-mms-error-activate-context but the status is still in Pending state

This one is on me. In telephony-service/handler/texthandler.cpp's TextHandler::RedownloadMessage function I've set the message to pending, then sent method call to nuntium via dbus and didn't check if it arrived and worked correctly.

I will fix it, when i have my issues sorted (and someone didn't fix it before me 😉 ).

[jackmcdougal]

@jezek I think having 40+ commits is still better for review than 84. If possible, squash changes that cancel each other if any. I see you added a bunch of new logging messages with "jezek" tag - those probably should be either prettified to look same with rest of debug output or removed.

On unrelated note, I came here trying to debug push parsing issue with Tele2 SE: https://paste.ubuntu.com/p/QhCd57JGsB/. It seems to fail with Download issues: org.freedesktop.DBus.Error.InvalidArgs: Invalid URL: '' on both original and your rewrite, so not directly related, but maybe you have an idea what could be wrong given the scope of changes.

I'm having this issue too (org.freedesktop.DBus.Error.InvalidArgs: Invalid URL: '' ). Dev / Pixel3a running PR_ofono_28 (testing some APN issue fixes). Outbound mms are working, my inbound fails with oops. If I grab the url from nuntium log, I can successfully wget the file. Any luck finding the source?
Also, I recently installed dev from the installer to salvage a corrupt system ... but ubports-qa list shows:
sudo ubports-qa list
PR_ofono_28 android9
... not sure if it is normal I have the android0 in there ...

Thanks!

PS: sorry for putting this here ... I know it doesn't have anything to do with this PR as I don't have re-download option.... but not sure where else to go?

[fredldotme]

Since we have no one available doing an actual Go code review, but it is piece of a puzzle, I'd readily take the heat in case things go wrong. When the things in other places are taken care of I'd gladly merge it.