Add a pathStrip option to the react native plugin

[nevir]

This allows for the user to customize how raven normalizes file URIs. For example, to support a Code Push bundle in addition to the baked in one:

Raven.addPlugin(require('raven-js/plugins/react-native'), {
  // Strip all paths up to the last /, resulting in paths like
  // `/main.jsbundle`, `/app.jsbundle`, etc.
  pathStrip: /^.*(?=\/)/,
});

[benvinegar]

Cool. My only comment is: can we come up with a better name than preventDefault. That name is so synonymous with DOM events that I'd prefer we avoided it.

Some quick ideas:

• { rethrow: true }
• { useDefaultExceptionHandler: true }

[nevir]

rethrow is a good'un

Should/can I change default behavior to not rethrow as part of this as well?

[benvinegar]

Maybe. Another possibility (to fix #468) is we verify the request is sent before calling the original error handler. I'm not sure if that's a good idea or not though – e.g. if the crash takes ~500ms to actually appear / manifest in the app.

[nevir]

Alright, I'll stick with just the option for now, that can be layered on later

[benvinegar]

Well, I like rethrow if the default behavior is not to rethrow :) So maybe your suggestion (flipping the default back) is fine. I just need another opinion from a React Native developer to chime in here.

[nevir]

Well, I can do the options.rethrow === false approach (e.g. rethrow: true being the default) if the name sticks :P

[nevir]

Here's the easy way out of noRethrow

[nevir]

Aha, ok. So, after further testing, iOS react's dev mode does use the global exception handler to display error redboxes.

It seems like the better behavior here is for default behavior to rethrow in dev mode (for redbox), and not to rethrow otherwise (to prevent the exception from bubbling out). That preserves the behavior of raven-js 1.x, at least

---

Pushed an update with that behavior

[benvinegar]

@nevir – Hey, just a heads up I was out for vacation but I'm back to look at this now.

I just want to fire up this patch and verify the behavior personally before accepting. Probably by EOD tomorrow.

[benvinegar]

If rethrow is false, this will still throw in __DEV__.

How about:

var rethrow = 'rethrow' in pluginOptions ? pluginOptions.rethrow || __DEV__;

[benvinegar]

@nevir – btw, now that we've figured out what the default behavior should be (with __DEV__), is there still a need for the rethrow option?

[nevir]

Probably not! updating to cut that out

[nevir]

Updated - PTAL

[nevir]

I'm unsure how to appropriately test the pathSplit option without a pretty substantial refactor (or only testing _normalizeData in isolation, which misses most of it) - so skipped a test there :S

[benvinegar]

@nevir – first off, thanks for sticking with this. Will definitely merge in.

One last question re: __DEV__ and the global exception handler: if Raven.js is absent in a React Native app, and an exception is thrown, does it not call the global handler / crash the app?

[nevir]

Uncaught JS exceptions do crash the app by default (i.e. including Raven prevents that)

want me to bring the option back? :P

[nevir]

The problem is that if the app is allowed to crash, the exception will be never caught.

Or Raven could defer the default handler until the exception is reported, but you risk additional state corruption; there's no way to pause the rest of the VM

[nevir]

(A better solution might be to have react-native support in the ObjC and Java clients; but that sounds thorny)

[benvinegar]

If you don't mind, let's pull out your __DEV__/global exception handler changes and put that in another PR where we can continue this conversation. I don't want to hang up your pathStrip changes, which I think are separate.

(If this is a pain in the ass I can do it for you.)

[benvinegar]

I understand the predicament where the app crashes before the HTTP request can go out ... but I'm not sure we should just swallow the exception entirely. Maybe waiting for the HTTP response (w/ a short timeout) is best (e.g. rethrow in .5 secs).

[nevir]

Sounds good, updated to just that commit

[benvinegar]

👍

[nevir]

Added #533 to kickstart the conversation