formatjs / dust-intl

Dust helpers for internationalization.
http://formatjs.io/dust/
Other
48 stars 11 forks source link

throw exception when ctx is missing params._key and improve error message #58

Open deddu opened 8 years ago

deddu commented 8 years ago

If the params._key lookup fails, the message is not really helpful for debugging.

here's a typical error message with stacktrace:

[DUST:ERROR] Error in helper `formatMessage`: A message must be provided as a String or AST.
[DUST:ERROR] Rendering failed with error `TypeError: A message must be provided as a String or AST.`
[DUST:ERROR] Error in helper `formatMessage`: A message must be provided as a String or AST.
TypeError: A message must be provided as a String or AST.
    at new MessageFormat (/Users/web-home/node_modules/intl-messageformat/lib/core.js:21:15)
    at /Users/web-home/node_modules/dust-intl/node_modules/intl-format-cache/lib/memoizer.js:16:22
    at Object.formatMessage (/Users/web-home/node_modules/dust-intl/lib/helpers.js:237:21)
    at Chunk.helper (/Users/web-home/node_modules/dustjs-linkedin/lib/dust.js:811:33)
    at body_0 (evalmachine.<anonymous>:1:2965)
    at /Users/web-home/node_modules/adaro/lib/patch/index.js:89:33
    at /Users/web-home/node_modules/adaro/lib/reader/js.js:39:13
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:380:3)

As you can see the stacktrace is not mentioning any file of my project. The reason for this is pretty simple though: is a simple key missing.

Clearly this is happening because the message is undefined.

Let me expose a little more details:

in dust-intl/.../helpers.js in the function formatMessage, there's the following lookup of params._key in the context.

 src$utils$$.contextGet(context, ['intl', 'messages', src$utils$$.tap(params._key, chunk, context)]);

if the key is missing in the context, the value assigned to msg is undefined. Which causes a failure later on when calling

formatter = getMessageFormat(msg, locales, formatOptions);

Naively, I would suggest adding a check and an exception in line 235. I was thinking that a simple check on the value could do it. EG: something trivially simple like

            if (!msg){
                  throw new TypeError('failed key lookup: ' + params._key);
            }

What you think? I'll be glad to submit a pull request if you find that appropriate. This will make debugging my (poor) code dramatically simpler!

normtronics commented 8 years ago

+1 This would help me as well

leachiM2k commented 8 years ago

Hey, +1, but I would also like to have an default value for not found keys.

Would it be helpful for you, if I'd provide a patch for this as a pull request?

ramiyav commented 5 years ago

+1 Would appreciate a fix for this. We had to spend a couple of hours digging into this error. Specifying the key as a part of the error, will be very helpful. Thank you.