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

Add json-stringify-safe to avoid circular JSON references #652

Merged
merged 1 commit into from
Jul 27, 2016

Conversation

mattrobenolt
Copy link
Contributor

@mattrobenolt mattrobenolt commented Jul 13, 2016

@benvinegar any idea how I could write some tests for this? Everything seems to stub out _makeRequest which is what calls stringify, and I don't want to make an actual XHR request. Also unittesting the vendored code is probably moot. But this does as advertises.

Fixes GH-651


This change is Reviewable

@@ -0,0 +1,27 @@
exports = module.exports = stringify
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we use this as an actual dependency in package.json now? I don't even know how that works and we don't have any dependencies yet, so I opted to just vendor. Ideally, we'd just point to this package on npm.

Copy link
Contributor

Choose a reason for hiding this comment

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

var stringify = require('json-stringify-safe');

From the library docs, should work fine.

@momoadeli
Copy link

Thanks for this (I'm coming from issue # 651). Needless to say, there is indeed a cyclic reference in our rejection.config.context which is generating the circular reference error. This, ideally, should not disrupt raven.js but I can see the pros and cons of how this is addressed.

@mattrobenolt
Copy link
Contributor Author

Yeah, agreed. I don't think raven-js should blow up at all from that. Recording an error should not trigger its own errors. :)

@mattrobenolt mattrobenolt changed the title Vendor in json-stringify-safe to avoid circular JSON references Add json-stringify-safe to avoid circular JSON references Jul 14, 2016
@momoadeli
Copy link

momoadeli commented Jul 18, 2016

@mattrobenolt, any thoughts on when this will be available via CDN? I'm currently on v3.2.1.

Many thanks.

@benvinegar
Copy link
Contributor

We'll get this out in a release either today or tomorrow. I still think you should look into removing this cyclical reference rather than relying on this implementation, though.

@momoadeli
Copy link

@benvinegar, thanks for the reply. Point taken. Until then, do you have a timeline for release? Apologies for being repetitive--as developers ourselves we understand the demands you're under. Thanks!

@benvinegar benvinegar merged commit a912ddf into master Jul 27, 2016
@benvinegar benvinegar deleted the stringify branch July 27, 2016 02:45
@benvinegar
Copy link
Contributor

@megabyzus – sorry, I was out on vacation for a bit and couldn't get to this in time. Prioritizing now.

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.

3 participants