Rocket.Chat icon indicating copy to clipboard operation
Rocket.Chat copied to clipboard

fix: Handle errors properly when loading Moment Locale

Open nmagedman opened this issue 8 months ago • 2 comments

Proposed changes (including videos or screenshots)

  1. BUGFIX: Use await when calling async function Assets.getTextAsync()

    Commit df82bccb86fdab509c9de324ad4adfda93f0ddc1 "chore: Use Assets’ async API (#29311)" replaced several Assets.getText() calls with await Assets.getTextAsync(). One of those getTextAsync calls was not awaited.

    That one was especially problematic in that it is in a try/catch block, however you cannot try/catch a Promise, so we missed handling that error. You must either try/catch the awaited Promise or chain on a Promise#catch handler.

  2. Meteor method loadLocale: Allow getMomentLocale() exceptions to pass-through

    Do not try/catch getMomentLocale():

    • It is an async function. You cannot try/catch Promises, so this try/catch has been a NOOP for years already. We apparently don't need it.
    • getMomentLocale() already handles exceptions and raises an (almost?) identical exception already.

Issue(s)

Steps to test or reproduce

  • Configure Log_Exceptions_to_Channel with a channel name
  • In user preferences, change the Language to "Norwegian Bokmål (Norway)"
  • In the exceptions channel, it will say:
    Exception while invoking method loadLocale
    > Error: Unknown asset: moment-locales/nb-no.js
    
  • Change the language to Default
  • Apply the PR
  • Change the language back again to "Norwegian Bokmål (Norway)"
  • loadLocale will successfully load the nb.js Moment Locale file (sans -no country)

Further comments

nmagedman avatar Jun 10 '25 18:06 nmagedman

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

dionisio-bot[bot] avatar Jun 10 '25 18:06 dionisio-bot[bot]

⚠️ No Changeset found

Latest commit: c8f03cfb8374640615ccf987e16bf121d52ea9d1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Jun 10 '25 18:06 changeset-bot[bot]