Skip to content

Conversation

gregorylegarec
Copy link
Contributor

@gregorylegarec gregorylegarec commented Aug 24, 2017

I barely understand how it happened, but RavenJS stored breadcrumbs with undefined data.url.

It caused the truncate() method to throw an error having for consequence that our Exceptions were never logged into Sentry.

Here is a temporary workaround that I made in our app until a fix will be made.

Another solution should have been to prevent undefined str directly in truncate().

for (var j = 0; j < urlProps.length; ++j) {
urlProp = urlProps[j];
if (data.hasOwnProperty(urlProp)) {
if (data.hasOwnProperty(urlProp) && data[urlProp]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only problem with this is that it will skip empty strings. It's probably safer to explicitly check for undefined. Or alternatively, change truncate to not explode on undefined values.

Copy link
Contributor Author

@gregorylegarec gregorylegarec Aug 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first thought was actually to update truncate. I may amend my commit.

By the way, an empty string does not need to be truncated, and testing falsy values also handle null. So let me know what your preference is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, an empty string does not need to be truncated

Good point. Okay, I mean given that there are tests, we can proceed as-is.

@benvinegar benvinegar merged commit e5a6e72 into getsentry:master Aug 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants