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

Support Node >= 1.0 and io.js. #192

Conversation

markstos
Copy link
Contributor

io.js is out now and although it is version 1.0.1, trying to require
'newrelic' with it was crashing:

 message = "New Relic for Node.js requires a version of Node equal to or\n" +
           "greater than 0.6.0. Not starting!"

There was a bug in a version checking that was only checking that
the minor version was less than 6, but was ignoring the major version,
so it failed to notice that that 1.0.1 > = 0.6.0.

Before and after checking was done by loading the iojs REPL and trying
this:

var nr = require('./index.js');

This crashes before the change and works after it.

Ref: https://iojs.org/

@asilvas
Copy link
Contributor

asilvas commented Jan 15, 2015

+1 after minor fix. We're testing io.js as well.

@@ -13,7 +13,9 @@ try {
logger.debug("Process was running %s seconds before agent was loaded.",
process.uptime())

if (process.version && process.version.split('.')[1] < 6) {
var major = process.version.split('.')[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

parseInt

@markstos
Copy link
Contributor Author

I found a second bug here. The code checks for > 0.6.0, but the README says: "New Relic for Node is only supported on Node.js 0.8 and newer". I presume the README is correct here.

io.js is out now and although it is version 1.0.1, trying to require
'newrelic' with it was crashing:

     message = "New Relic for Node.js requires a version of Node equal to or\n" +
               "greater than 0.6.0. Not starting!"

There was a bug in a version checking that was only checking that
the minor version was less than 6, but was ignoring the major version,
so it failed to notice that that 1.0.1 > = 0.6.0.

Before and after checking was done by loading the iojs REPL and trying
this:

   var nr = require('./index.js');

This crashes before the change and works after it.

Ref: https://iojs.org/
@markstos markstos force-pushed the support-newrelic-greater-than-1-and-iojs branch from bd7c7ae to a946516 Compare January 15, 2015 02:56
@markstos
Copy link
Contributor Author

I've amended the first commit to add parseInt-- thanks for that catch. I've added a second commit to bump the version requirement to 0.8.0 to match the README. I presume you wanted them to agree.

@hayes
Copy link
Contributor

hayes commented Jan 15, 2015

Thanks for contributing this change. We are working to properly support io.js (and node 0.12) and we know there are parts of the agent that will not work yet. 1.0 of io.js is still an alpha (similar to 0.11.15 which is an rc for node 0.12), so we will not officially support it until it has a release that is considered stable by the maintainers of the project. That being said, the agent being limited to node version in the 0.x range is a bug that this PR would fix. We want to also allow people to run the agent on 0.6 as well, we do not provide support or guarantee functionality on 0.6, but most of the agent should work without any issues, so we do not feel it makes sense to artificially restrict its usage.

I'll try to get this merged and included in today's release.

@hayes
Copy link
Contributor

hayes commented Jan 15, 2015

We like to credit community contribution in our release notes, do you have a preferred way for us refer to you (most people usually use their github name, or full name).

@markstos
Copy link
Contributor Author

My preferred reference is "Mark Stosberg". Thanks!
On Jan 15, 2015 5:51 PM, "Michael Hayes" notifications@github.com wrote:

We like to credit community contribution in our release notes, do you have
a preferred way for us refer to you (most people usually use their github
name, or full name).

Reply to this email directly or view it on GitHub
#192 (comment)
.

@hayes
Copy link
Contributor

hayes commented Jan 19, 2015

released in 1.14.7

@hayes hayes closed this Jan 19, 2015
@markstos
Copy link
Contributor Author

Thanks!

bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 19, 2024
…8ad22aa8d304deedb6

[Snyk] Security upgrade newrelic from 10.0.0 to 10.3.1
bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 23, 2024
…8ad22aa8d304deedb6

[Snyk] Security upgrade newrelic from 10.0.0 to 10.3.1
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.

None yet

3 participants