Conventionalize to fix sneaky breadcrumb logging bugs

[cooperka]

This PR makes the code follow more standard JS conventions, fixing several major bugs related to breadcrumb logging. It also tackles a few other cleanup items related to the changes.

1. Properly log objects, booleans, null, and undefined in breadcrumbs
   • Previously, all of these values were completely dropped without warning
2. Use typeof in several places instead of various alternatives
   • See here for more info
   • Fixes a few potential bugs with inheritance and prototype modification
3. Use regular functions instead of closures (previously 22aac3a)
4. Check hasOwnProperty before adding to typedMap output
5. Output slightly more helpful warnings
6. Ignore the .idea directory created by IntelliJ IDEs

Please see the individual commits for more information and to understand each change as a unit.

---

Obsolete:

Note: None of these changes should be breaking, except in the following case. The use of regular functions instead of closures means that this can refer to the wrapper object in rare cases, for example if you do the following:

const bugSnag = new BugSnag(API_KEY);
export const bugSnagWrapperUtil = {
    trackError: bugSnag.notify,
};

This is generally a desirable side-effect of using ES6 classes, and can easily be resolved by using a closure to maintain the original this:

    trackError: (...params) => bugSnag.notify(...params),

[Andreyco]

Probably you should put type of name? That would be helpful.

[cooperka]

I'd be happy to change this if I can make the message more helpful, but I think the variable itself is more useful than just the type of the variable. You can intuit the type from the content, but not vice versa.

One example of how this could be confusing is if name is null, then typeof name is 'object'.

[cooperka]

@kattrali is there anything I can do to help this get merged? It fixes multiple important bugs. Let me know :)

[kattrali]

Thanks for the bump, @cooperka. It's a little hard to bring all of this in at once, since it doesn't seem to all be breadcrumb related. I just have a few questions.

1. Properly log objects, booleans, null, and undefined in breadcrumbs
   Previously, all of these values were completely dropped without warning

Makes sense. Though it seems a bit odd to add the logged error message as metadata. A console warning could suffice, similar to the other errors which could be encountered.

2. Use typeof in several places instead of various alternatives
   Fixes a few potential bugs with inheritance and prototype modification

What behavior does this fix?

3. Use regular functions instead of closures

This change was introduced in #17, since requiring something like trackError: (...params) => bugSnag.notify(...params) was not immediately obvious to users of the library, and the resulting error was opaque. I encountered it when using Promise.catch(bugsnag.notify). This seems a bit unrelated to the breadcrumb issues.

[cooperka]

@kattrali thanks for the feedback. My apologies for the confusion, my intent here really is to conventionalize the code in order to fix a few sneaky bugs, rather than fixing them more directly with workarounds. I've updated the title to make it more clear. If you'd like me to split it into separate PRs I would be happy to.

I didn't realize the closures were added intentionally; I've rebased to exclude that commit. It does make sense why users were confused by the behavior you mentioned.

Can you clarify what you mean by "it seems a bit odd to add the logged error message as metadata"? I see, yeah that's weird I'll fix that haha.

Finally, there are a few answers on this SO question that explain why I prefer typeof. Most importantly for me is that instanceof compares to an existing instance directly, such as Object, which may not always be the same depending on scope. Obviously it's true that Object === Object, but a var saved by someone else's code may extend from a different Object than the Object in the current global scope. Let me know if that makes sense.

Also, in one case !metadata instanceof Object always evaluated to false because !metadata is a boolean.

[kattrali]

Gotcha! Thanks for the clarifications, its much clearer now what is going on. I'll look it over again.

[foxyblocks]

lgtm 🦉

[kattrali]

Thanks!