Fix timezone crash #8485

larkox · 2025-01-16T16:38:15Z

Summary

Fix crash that occurs on several timezones.

Ticket Link

Fix https://mattermost.atlassian.net/browse/MM-62298
Fix #8423

Release Note

Fix crash when using certain timezones

rahimrahman

I have 1-2 non-blocking suggestions. Thanks for getting to this so quickly.

rahimrahman · 2025-01-16T17:29:20Z

app/components/formatted_date/index.tsx

+            timeZone,
+        }).format(new Date(value));
+    } catch (error) {
+        logDebug('Failed to format date', {locale, timezone}, error);


Shouldn't this be logError? This would have cause a crash if wasn't caught.

It would add entry into sentry with proper Error category.

Suggested change

logDebug('Failed to format date', {locale, timezone}, error);

logError('Failed FormattedDate to format date', {locale, timezone}, error);

@rahimrahman Would it be better to check if the timeZone name is valid first, and maybe default to the server's timezone (or something), rather than try catching?

@cpoile 0/5. I'm okay with the try..catch. Just asking about the logError vs logDebug.

With the try..catch, it won't crash anymore so doing the Intl.DateTimeFormat would validate all the props.

rahimrahman · 2025-01-16T17:29:37Z

index.ts

@@ -46,7 +46,7 @@ if (global.HermesInternal) {
    require('@formatjs/intl-pluralrules/polyfill-force');
    require('@formatjs/intl-numberformat/polyfill-force');
    require('@formatjs/intl-datetimeformat/polyfill-force');
-    require('@formatjs/intl-datetimeformat/add-golden-tz');
+    require('@formatjs/intl-datetimeformat/add-all-tz');


What's the diff between golden and all?

Looked up, besides the obvious:

Golden: contains popular set of timezones from IANA database

rahimrahman · 2025-01-16T17:31:28Z

app/components/formatted_date/__snapshots__/index.test.tsx.snap

@@ -1,5 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

+exports[`<FormattedDate/> should default when timezone is not found 1`] = `


This is the only time I'm okay with snapshots!

Indeed -- unit tests that actually test functionality :) 👍

rahimrahman · 2025-01-16T17:35:14Z

app/components/formatted_date/index.tsx

+                ...format,
+            }).format(new Date(value));
+        } catch (error) {
+            logDebug('Failed to format default date', {locale}, error);


Same.

Suggested change

logDebug('Failed to format default date', {locale}, error);

logError('Failed FormattedDate to format default date', {locale, timezone}, error);

cpoile

I'll approve conditional on Rahim's comments. Also assuming that you've checked (and wrapped as needed) all other uses of DatTimeFormat as well?
Awesome, thanks @larkox !

cpoile · 2025-01-16T20:12:22Z

app/components/formatted_date/__snapshots__/index.test.tsx.snap

@@ -1,5 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

+exports[`<FormattedDate/> should default when timezone is not found 1`] = `


Indeed -- unit tests that actually test functionality :) 👍

cpoile · 2025-01-16T20:17:37Z

app/components/formatted_date/index.tsx

+            timeZone,
+        }).format(new Date(value));
+    } catch (error) {
+        logDebug('Failed to format date', {locale, timezone}, error);


@rahimrahman Would it be better to check if the timeZone name is valid first, and maybe default to the server's timezone (or something), rather than try catching?

lindalumitchell

Verified fix on PR build for Android and iOS. Verified setting the timezone to previously crashing timezones and then accessing channels did not crash the app. Also ran through quick smoke tests of general functionality including SSO and push notifications. No issues found. 👍

amyblais · 2025-01-17T06:29:28Z

/cherry-pick release-2.24

mattermost-build · 2025-01-17T06:29:30Z

Cherry pick is scheduled.

amyblais · 2025-01-17T06:29:32Z

/cherry-pick release-2.25

mattermost-build · 2025-01-17T06:29:33Z

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
From github.com:mattermost/mattermost-mobile
 * [new branch]          actions-websockets-tests -> upstream/actions-websockets-tests
   d1d3fda83..592b3805f  main         -> upstream/main
   1b5ec1e00..5615405e3  release-2.25 -> upstream/release-2.25
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-mattermost-mobile-#8485-upstream-release-2.24-1737095371
Switched to a new branch 'automated-cherry-pick-of-mattermost-mobile-#8485-upstream-release-2.24-1737095371'
Branch 'automated-cherry-pick-of-mattermost-mobile-#8485-upstream-release-2.24-1737095371' set up to track remote branch 'release-2.24' from 'upstream'.

+++ About to attempt cherry pick of PR #8485 with merge commit 592b3805f1240794915c1acbd7819f0238fc4abe.

Auto-merging assets/base/i18n/en.json
Auto-merging package-lock.json
CONFLICT (content): Merge conflict in package-lock.json
Auto-merging package.json
CONFLICT (content): Merge conflict in package.json
error: could not apply 592b3805f... Fix timezone crash (#8485)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

+++ Conflicts detected:

UU package-lock.json
UU package.json
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the master branch and cleaning up.

mattermost-build · 2025-01-17T06:29:34Z

Cherry pick is scheduled.

mattermost-build · 2025-01-17T06:29:37Z

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-mattermost-mobile-#8485-upstream-release-2.25-1737095374
Switched to a new branch 'automated-cherry-pick-of-mattermost-mobile-#8485-upstream-release-2.25-1737095374'
Branch 'automated-cherry-pick-of-mattermost-mobile-#8485-upstream-release-2.25-1737095374' set up to track remote branch 'release-2.25' from 'upstream'.

+++ About to attempt cherry pick of PR #8485 with merge commit 592b3805f1240794915c1acbd7819f0238fc4abe.

Auto-merging package-lock.json
CONFLICT (content): Merge conflict in package-lock.json
error: could not apply 592b3805f... Fix timezone crash (#8485)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

+++ Conflicts detected:

UU package-lock.json
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the master branch and cleaning up.

* Fix timezone crash by adding all timezones to the polyfill * update snapshots; be careful about undefined: diff on local vs CI * one more that's different locally vs CI * i18n --------- Co-authored-by: Christopher Poile <cpoile@gmail.com>

larkox added 3 commits January 16, 2025 16:48

Fix timezone crash by adding all timezones to the polyfill

9b10bc0

Merge branch 'main' into fixTimezoneCrash

aa006ec

Merge branch 'main' into fixTimezoneCrash

46625b3

larkox added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jan 16, 2025

larkox requested review from a team and cpoile and removed request for a team January 16, 2025 16:38

mm-cloud-bot added release-note labels Jan 16, 2025

larkox requested a review from rahimrahman January 16, 2025 16:38

rahimrahman approved these changes Jan 16, 2025

View reviewed changes

cpoile approved these changes Jan 16, 2025

View reviewed changes

cpoile added 2 commits January 16, 2025 16:25

update snapshots; be careful about undefined: diff on local vs CI

20f2859

one more that's different locally vs CI

7ec02e2

lindalumitchell added the Build Apps for PR Build the mobile app for iOS and Android to test label Jan 16, 2025

i18n

9908088

lindalumitchell added Build Apps for PR Build the mobile app for iOS and Android to test and removed Build Apps for PR Build the mobile app for iOS and Android to test labels Jan 16, 2025

jwilander requested a review from lindalumitchell January 16, 2025 21:57

lindalumitchell approved these changes Jan 16, 2025

View reviewed changes

lindalumitchell removed Build Apps for PR Build the mobile app for iOS and Android to test 3: QA Review Requires review by a QA tester labels Jan 16, 2025

jwilander merged commit 592b380 into main Jan 16, 2025
15 checks passed

jwilander deleted the fixTimezoneCrash branch January 16, 2025 22:29

jwilander added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Jan 16, 2025

amyblais added this to the v2.24.0 milestone Jan 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix timezone crash #8485

Fix timezone crash #8485

larkox commented Jan 16, 2025

rahimrahman left a comment

rahimrahman Jan 16, 2025

cpoile Jan 16, 2025

rahimrahman Jan 16, 2025

rahimrahman Jan 16, 2025

rahimrahman Jan 16, 2025

rahimrahman Jan 16, 2025

cpoile Jan 16, 2025

rahimrahman Jan 16, 2025

cpoile left a comment

cpoile Jan 16, 2025

cpoile Jan 16, 2025

lindalumitchell left a comment

amyblais commented Jan 17, 2025

mattermost-build commented Jan 17, 2025

amyblais commented Jan 17, 2025

mattermost-build commented Jan 17, 2025

mattermost-build commented Jan 17, 2025

mattermost-build commented Jan 17, 2025

	logDebug('Failed to format date', {locale, timezone}, error);
	logError('Failed FormattedDate to format date', {locale, timezone}, error);

		@@ -1,5 +1,11 @@
		// Jest Snapshot v1, https://goo.gl/fbAQLP

		exports[`<FormattedDate/> should default when timezone is not found 1`] = `

	logDebug('Failed to format default date', {locale}, error);
	logError('Failed FormattedDate to format default date', {locale, timezone}, error);

Fix timezone crash #8485

Fix timezone crash #8485

Conversation

larkox commented Jan 16, 2025

Summary

Ticket Link

Release Note

rahimrahman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpoile left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lindalumitchell left a comment

Choose a reason for hiding this comment

amyblais commented Jan 17, 2025

mattermost-build commented Jan 17, 2025

amyblais commented Jan 17, 2025

mattermost-build commented Jan 17, 2025

mattermost-build commented Jan 17, 2025

mattermost-build commented Jan 17, 2025