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

IntlError thrown in createFormatter, ignoring onError #1226

Closed
3 tasks done
jthrilly opened this issue Jul 29, 2024 · 4 comments
Closed
3 tasks done

IntlError thrown in createFormatter, ignoring onError #1226

jthrilly opened this issue Jul 29, 2024 · 4 comments
Labels
bug Something isn't working unconfirmed Needs triage.

Comments

@jthrilly
Copy link

jthrilly commented Jul 29, 2024

Description

Hi!

First of all, I want to thank you for producing such an amazing tool for the community.

Environment is NextJs 14.2.5.

I am working on a very fast moving project, which is churning through a lot of front end changes that most often do not have translations of any kind available yet (since it won't make sense until the parameters are set).

The lack of available messages creates a very noisy node log and developer console, because errors are logged via console.error, which of course means that a stack trace is printed too. This means every single missing translation produces 20+ lines. Needless to say, this makes debugging pretty tedious.

To quiet this down, I implemented an onError handler in my i18n.ts, which allows me to customize the error notification to something more subtle. This works perfectly.

However, on a hard page reload, an IntlError is thrown by createFormatter here:

This seems to be the only place in the code that the onError callback is used, but the error is still thrown.

It would be great if this throw could be removed, so that the user could decide entirely how they want to handle errors in the callback.

The way this is handled elsewhere is that the IntlError is constructed, but passed to onError. I could make that change here in a PR?

Alternatively, perhaps a global configuration for setting the error reporting level could be useful?

Thanks again!

Verifications

  • I've verified that the problem I'm experiencing isn't covered in the docs.
  • I've searched for similar, existing issues on GitHub and Stack Overflow.
  • I've compared my app to a working example to look for differences.

Mandatory reproduction URL

https://github.com/complexdatacollective/studio

Reproduction description

See above.

Expected behaviour

It would be amazing if createFormatted could not throw the error (or perhaps throw it only if an onError handler isn't defined?).

@jthrilly jthrilly added bug Something isn't working unconfirmed Needs triage. labels Jul 29, 2024
@amannn
Copy link
Owner

amannn commented Aug 14, 2024

Hey @jthrilly, thank you so much for the kind words—really happy to hear if next-intl works well for you! 🙂

I tried to run the repo you've linked to, but it seems like this is not possible without environment parameters.

Regarding the line you've linked to:

It's true that an error is thrown there, but if you check the call site of resolveFormatOrOptions below:

let options;
try {
options = resolveFormatOrOptions(typeFormats, formatOrOptions);
} catch (error) {
return getFallback();
}

The thrown error only results in the fallback being returned. So the error should in fact be handled.

Are you sure the error originates from that place?

A guess from my side is, that you might not have onError configured for usage on the client side (via NextIntlClientProvider), only on the server side (via i18n.ts).

If in doubt, please review the error handling docs (esp. the last paragraph when you have i18n.ts selected). Note that I've added 88dfb10 which might provide more clarity for this case. As a side note, I know the situation with having to forward onError manually is far from ideal, I'm planning to simplify this as part of #779.

Coming back to the topic, I definitely agree with you that all error logging should be handled via onError. It would be helpful if we could further isolate where the logging in your app is coming from.

@jthrilly
Copy link
Author

Thanks so much for your help! This turned out to be the issue.

I completely missed the last paragraph, and because the code snippets are behind tabs, I assumed that I only needed one or the other. Totally my mistake.

@amannn
Copy link
Owner

amannn commented Aug 15, 2024

No worries at all! The currently required configuration is rather cumbersome and makes it too easy to get it wrong.

Since the initial RSC release of next-intl, instrumentation.ts was added to Next.js. I think that can work as a better alternative to what's currently available.

E.g.:

// instrumentation.ts
import { onError } from 'next-intl';
 
export function register() {
  onError((error) => {
    // This is registered in all environments (RSC, Client Components)
  }); 
}

I need to investigate a bit more, but I think that will be a significantly better alternative for the future.

@amannn
Copy link
Owner

amannn commented Aug 26, 2024

I think we need some support from Next.js side on this one. I've created #1285 to track this, and opened a discussion on the Next.js side: vercel/next.js#69294.

If you're affected by this, feel free to chime in in the Next.js discussion and potentially upvote the discussion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working unconfirmed Needs triage.
Projects
None yet
Development

No branches or pull requests

2 participants