Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250 Internal] Notifications not being received #13392

Closed
Julesssss opened this issue Dec 7, 2022 · 22 comments
Closed

[$250 Internal] Notifications not being received #13392

Julesssss opened this issue Dec 7, 2022 · 22 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@Julesssss
Copy link
Contributor

Julesssss commented Dec 7, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Problem

Multiple platforms are not receiving notifications in the production/staging environment.

Platform:

Where is this issue occurring?

  • Android ✅
  • iOS ✅
  • Web?
  • Desktop?

Version Number: 1.2.33-7 1.2.34-1 1.2.35-0, 1.2.36-2,
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester (no customers): ALL
Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1670408581155829

View all open jobs on GitHub

@Julesssss Julesssss added Engineering Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Bug Something is broken. Auto assigns a BugZero manager. labels Dec 7, 2022
@Julesssss Julesssss self-assigned this Dec 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 7, 2022

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@Julesssss
Copy link
Contributor Author

I tested release builds as far back as 1.2.33-7 and I'm not receiving notifications in any build. 1.2.34-1 is where my Airship config change was merged and this was my first guess, but it seems this is unlikely to be the issue now.

Next guess is a backend change, which I'll look into next.

@Julesssss
Copy link
Contributor Author

Given that this already is on prod it should not be a deploy blocker, but it is a major issue that needs to be resolved ASAP

@Julesssss
Copy link
Contributor Author

interestingly I did receive a single push notification from Chronos on the latest prod build. Not receiving messages from other accounts though:

Screenshot_20221207_110727

@Julesssss
Copy link
Contributor Author

Julesssss commented Dec 7, 2022

I built a debuggable release build and discovered that we're hitting this condition. It makes sense that I'm not seeing notifications, but it does seem odd that this wasn't previously an issue. This is for local notification though which only affects web/desktop. I still need to figure out why Airship notifications have stopped.

Screenshot 2022-12-07 at 17 34 00

@Julesssss
Copy link
Contributor Author

Julesssss commented Dec 8, 2022

@robertjchen reports that he isn't receiving notifications on Android or Desktop. Perhaps this isn't just Android, I'll ask in Slack about iOS.

@0xmiros
Copy link
Contributor

0xmiros commented Dec 8, 2022

Same to me. I haven't received any notifications recently even on web localhost. I remember this was working before.
If this is open to external, I will try to find regression and propose solution.

@Julesssss
Copy link
Contributor Author

Same to me. I haven't received any notifications recently even on web localhost. I remember this was working before. If this is open to external, I will try to find regression and propose solution.

Thanks, but no this needs to be internal. It's critical, and is likely a backend issue.

@Julesssss Julesssss changed the title Android notifications not being received Notifications not being received Dec 8, 2022
@Julesssss
Copy link
Contributor Author

Focusing on the notification project in the meantime, which is more critical.

@melvin-bot melvin-bot bot added the Overdue label Dec 12, 2022
@Julesssss Julesssss added Weekly KSv2 and removed Daily KSv2 labels Dec 12, 2022
@melvin-bot melvin-bot bot removed the Overdue label Dec 12, 2022
@Julesssss
Copy link
Contributor Author

We discussed this not being a deploy blocker and will fix after the notification doc has been released.

@Julesssss
Copy link
Contributor Author

Looking into these PRs:

I can confirm this is a recent config change issue, as I can receive notifications when directly targeting my device from the Airship console:

Screenshot_20221212_160842

@Julesssss
Copy link
Contributor Author

Julesssss commented Dec 12, 2022

Next step is to dive into the logs. I found examples of successful and failed notifications.

Here's one of the failed messages. Notice that we route it via Pusher instead of Airship.

Screenshot 2022-12-12 at 17 04 18

Here are logs for a successful push notification send (from Chronos).


Theories:

  1. Have we rotated keys? I think no, because the test notification works, but I'm running out of ideas
  2. Are we somehow accidentally setting shouldFailToSend to true (we recently switched to staging)
  3. Send via Pusher async, OR just end via Pusher
    in Web-E because we're using the staging server?! We recently switched from prod to staging...
  4. A recent Web-E change. I've been through all PRs from the last week and those suspicious PRs mentioned above don't show an obvious bug
  5. Other PRs? A, B, C

@Julesssss

This comment was marked as off-topic.

@Julesssss Julesssss added the Reviewing Has a PR in review label Dec 12, 2022
@Julesssss
Copy link
Contributor Author

Just noting that my previous guesses were wrong 😅

I found the issue and a PR is in review

@Julesssss Julesssss added Daily KSv2 and removed Weekly KSv2 labels Dec 12, 2022
@Julesssss
Copy link
Contributor Author

Notifications are fixed -- just requires that users sign out and in again. Closing.

@parasharrajat
Copy link
Member

@Julesssss Am I eligible for reporting bonus here?

@Julesssss
Copy link
Contributor Author

Yeah of course. My bad.

@Christinadobrzyn could you please create an UpWork contract to pay a reporting bonus to @parasharrajat? Thanks

@Julesssss Julesssss reopened this Dec 13, 2022
@Christinadobrzyn
Copy link
Contributor

Upwork job - Internal - https://www.upwork.com/ab/applicants/1603246366811955200/job-details
External - https://www.upwork.com/jobs/~01cc93fd22b5099ef9

Hired @parasharrajat as reporter for the $250 payment.

@Julesssss or @parasharrajat can you confirm this job is finished and can be paid out/closed? I think it is based on the PR - https://github.com/Expensify/Web-Expensify/pull/35811

@Christinadobrzyn Christinadobrzyn changed the title Notifications not being received [250] Notifications not being received Dec 15, 2022
@Christinadobrzyn Christinadobrzyn changed the title [250] Notifications not being received [$250 Internal] Notifications not being received Dec 15, 2022
@Julesssss
Copy link
Contributor Author

Julesssss commented Dec 15, 2022

Hey @Christinadobrzyn, yep the PR was resolved and deployed.

@Christinadobrzyn
Copy link
Contributor

@parasharrajat when you have a moment, can you please accept the offer so I can pay you? Thanks!

https://www.upwork.com/jobs/~01cc93fd22b5099ef9

@parasharrajat
Copy link
Member

@Christinadobrzyn Accepted.

@Christinadobrzyn
Copy link
Contributor

Thanks @parasharrajat! I paid you as the reporter, closed the Upwork job and am closing this! Let me know if I've missed anything!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

4 participants