[HOLD for payment 2024-01-18] [$250] Dev: IOS - App crashes when opening a transaction report

[kbecciv]

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

---

Action Performed:

1. Open the ios app
2. Open an iou report that has transactions
3. Click on a transaction to open the transaction report

Expected Result:

App does not crash

Actual Result:

App crashes with console error: Render Error Invalid time value

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [ ] Android / native
• [ ] Android / Chrome
• [x] iOS / native
• [ ] iOS / Safari
• [ ] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: Dev 1.3.69.1 Reproducible in staging?: no Reproducible in production?: no If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/93399543/d5d81b9a-3fa4-4fdc-9c30-ae1b6d2b48c0

Expensify/Expensify Issue URL: Issue reported by: @tienifr Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694440628004359

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010d24045ce9cc8210
• Upwork Job ID: 1716907956821823488
• Last Price Increase: 2024-06-27
• Automatic offers:
• situchan | Reviewer | 28015222
• tienifr | Contributor | 28015224

Issue Owner

Current Issue Owner: @

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~011e5e93003759619c

[melvin-bot[bot]]

Triggered auto assignment to @NicMendonca (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)

[tienifr]

I reported this bug a long time ago here: https://expensify.slack.com/archives/C049HHMV9SM/p1693349618431379.

[tienifr]

Proposal

Please re-state the problem that we are trying to solve in this issue.

iOS - App crashes when opening a report

What is the root cause of that problem?

This is due to our migration from momentjs to date-fns library recently. One of the participants of that report may have a deprecated timezone in momentjs that is not supported in date-fns. In this case it is Etc/GMT-1 and Asia/Saigon. This is explained in momentjs documentation:

Some libraries use the ECMAScript Intl API for locales, time zones, or both. Some libraries still provide their own locale and time zone files like Moment and Moment-Timezone do.

date-fns belongs to the Intl group.

However, there might be differences in implementing this Intl across browsers/runtimes. Although they use the same Intl interface, some timezones are supported on one device but not the others. For example, I tried this code to validate a timezone:

Intl.DateTimeFormat(undefined, { timeZone: 'Asia/Saigon' })

but Mac Chrome and iOS yielded different results: success and error. This is a known issue of Hermes: https://github.com/facebook/hermes/issues/23. That's why the app only crashed on iOS but not Web. One more thing is that because momentjs supports almost all IANA timezones, the issue has not happend before.

There've been efforts to add polyfills for this Intl.DateTimeFormat interface for native here:

https://github.com/Expensify/App/blob/099fac6fcd3ec831070594403a2c259c9c40905f/src/libs/IntlPolyfill/index.native.js#L12

But that only implemented the core functions of DateTimeFormat, not the actual polyfills to support ECMA-402.

What changes do you think we should make in order to solve the problem?

Add support for all IANA timezones using formatjs (reference) for iOS platform:

require('@formatjs/intl-datetimeformat/polyfill-force');
require('@formatjs/intl-datetimeformat/add-all-tz');
require('@formatjs/intl-datetimeformat/locale-data/en');
require('@formatjs/intl-datetimeformat/locale-data/es');

[DylanDylann]

I also see this bug on the IOS App (production version). It seems this bug is serious

[NicMendonca]

@situchan proposal here for you! ^^

[NicMendonca]

bump @situchan ^

[tienifr]

FYI, this is the supported timezones on iOS and former momentjs.

[melvin-bot[bot]]

@NicMendonca, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[situchan]

reviewing today

[melvin-bot[bot]]

@NicMendonca, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[situchan]

@tienifr proposal looks promising but do you have clear repro step?

[tienifr]

@situchan You could try this:

1. Change your IP location by a VPN service (UrbanVPN is a great & free one) then switch to one of the affected locations (Vietnam for example)
2. Open Web app on Chrome
3. Settings >> Profile >> Timezones >> Turn on Automatically determine your location
4. Verify the automatic timezone is Asia/Saigon
5. Open iOS app, go to any chat participated in by the above account

[melvin-bot[bot]]

@NicMendonca, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[NicMendonca]

@situchan have you been able to try? ^^

[NicMendonca]

@situchan bumped you here: https://expensify.slack.com/archives/C02NK2DQWUX/p1696542533862449

[melvin-bot[bot]]

@NicMendonca, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[tienifr]

@situchan Another way (easier) is to use MacOS's Date & Time settings to manually change time zone.

[suneox]

Can not reproduce when same country may be fixed or we have other condition https://github.com/Expensify/App/assets/11959869/1422d4bb-a6e1-416e-9918-1d6805f75c61

[tienifr]

Still can reproduce. I'm not sure how you could get Asia/Saigon automatically on iOS. I'm testing on iOS 17.

[suneox]

I also using iOS 17, so I think it can depend on model I'm using ip 12 pro VN/A maybe load different firmware so VPN or change timezone not effective due to I'm also raise another issue but reviewer can not reproduce at here with same config. Or can you check response from API

[situchan]

I managed to reproduce following this step - https://github.com/Expensify/App/issues/27392#issuecomment-1738941311. @tienifr's proposal LGTM! 🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

Triggered auto assignment to @pecanoro, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[pecanoro]

Sounds good since we are already using format.js, assigning @tienifr

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[tienifr]

Since this one is the prerequisite to fix https://github.com/Expensify/App/issues/27927. I would combine them into one PR https://github.com/Expensify/App/pull/29113.

[situchan]

Since this one is the prerequisite to fix #27927. I would combine them into one PR #29113.

If so, you could put #27927 on hold until PR for this one is merged.

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.84-10 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

• https://github.com/Expensify/App/pull/29113

If no regressions arise, payment will be issued on 2023-10-23. :confetti_ball:

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• [ ] External issue reporter
• [ ] Contributor that fixed the issue
• [ ] Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @situchan requires payment
• @tienifr requires payment
• @tienifr requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot[bot]]

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.84-10 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

• https://github.com/Expensify/App/pull/29113

If no regressions arise, payment will be issued on 2023-10-23. :confetti_ball:

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• [ ] External issue reporter
• [ ] Contributor that fixed the issue
• [ ] Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @situchan requires payment
• @tienifr requires payment
• @tienifr requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~010d24045ce9cc8210

[melvin-bot[bot]]

Current assignee @situchan is eligible for the External assigner, not assigning anyone new.

[NicMendonca]

@situchan @tienifr can you both please apply to the job? https://www.upwork.com/jobs/~010d24045ce9cc8210

[situchan]

@NicMendonca This is not done yet. PR was reverted due to specific issue on MacOS Sonoma

I think we should open this back for proposals with that case covered. It seems @tienifr hasn't found solution yet.

[pecanoro]

@tienifr should we open this back to proposals?

[pecanoro]

I am returning this one back to the pool since haven't got any replies from @tienifr in more than a week

[tienifr]

Updated Proposal

Please re-state the problem that we are trying to solve in this issue.

iOS - App crashes when opening a report

What is the root cause of that problem?

This is due to our migration from momentjs to date-fns library recently. One of the participants of that report may have a deprecated timezone in momentjs that is not supported in date-fns. In this case it is Etc/GMT-1 and Asia/Saigon.

date-fns uses the ECMAScript Intl API for locales, time zones, or both. However, there might be differences in implementing this Intl across browsers/runtimes. Although they use the same Intl interface, some timezones are supported on one device but not the others.

There've been efforts to add polyfills for this Intl.DateTimeFormat interface for native here:

https://github.com/Expensify/App/blob/099fac6fcd3ec831070594403a2c259c9c40905f/src/libs/IntlPolyfill/index.native.js#L12

But that only implemented the core functions of DateTimeFormat, not the actual polyfills to support ECMA-402.

[Updated] About the regression

Link to Slack: https://expensify.slack.com/archives/C01GTK53T8Q/p1697212011581449

Actually the problem was not with Mac Sonoma (if it was then all current users would be affected regardless of the polyfills); but with the timezone abbreviations (i.e. WET). @formatjs does not recognize those abbreviations as valid timezone names while they should as confirmed in IANA's official tz database guideline.

What changes do you think we should make in order to solve the problem?

1. Add support for all IANA timezones using formatjs (reference):

require('@formatjs/intl-datetimeformat/polyfill-force');
require('@formatjs/intl-datetimeformat/add-all-tz');
require('@formatjs/intl-datetimeformat/locale-data/en');
require('@formatjs/intl-datetimeformat/locale-data/es');

2. Because JS Engines do not expose default timezone, the polyfill cannot detect local timezone that a browser is in (reference). We must manually do this by getting the local timezone before adding polyfill.
3. To handle the abbreviations issue, we should create our own list to map those backward timezones to modern format ones as suggested here.

WET: Europe/Lisbon
PST8PDT: America/Los_Angeles
MST: America/Phoenix
...

What alternative solutions did you explore? (Optional)

We can create an upstream PR for formatjs to fix step 3. Seems like their implementation does not comply with IANA standard.

[situchan]

@tienifr can you please share test branch? I now have Sonoma so able to valid your solution

[melvin-bot[bot]]

@pecanoro, @NicMendonca, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[pecanoro]

Waiting for @tienifr and @situchan to test the proposal

[pecanoro]

friendly bump!!!

[tienifr]

@situchan Draft PR here for testing: https://github.com/Expensify/App/pull/31074. ~For Chrome on Mac Sonoma, it will fallback to UTC because timezone API is broken there, we cannot hack around that. We only can avoid crashing.~

[tienifr]

Oh I'm sorry, the issue with Chrome on Sonoma has been resolved by Chrome team. Now we can test it easily.

[situchan]

@tienifr so you mean that original PR should work now?

[tienifr]

Actually the problem was not with Mac Sonoma (if it was then all current users would be affected regardless of the polyfills); but with the timezone abbreviations (i.e. WET). @formatjs does not recognize those abbreviations as valid timezone names while they should as confirmed in IANA's official tz database guideline.

The problem is not only with Sonoma so original PR won't fully work.

[NicMendonca]

@tienifr @situchan any update here?

[tienifr]

@NicMendonca I'm waiting for @situchan input on the test branch.

[situchan]

reviewing and testing

[melvin-bot[bot]]

@pecanoro, @NicMendonca, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!