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

7.x causes invalid range error (process.uptime) #5856

Closed
3 tasks done
Venipa opened this issue Sep 29, 2022 · 11 comments · Fixed by #5859
Closed
3 tasks done

7.x causes invalid range error (process.uptime) #5856

Venipa opened this issue Sep 29, 2022 · 11 comments · Fixed by #5859

Comments

@Venipa
Copy link

Venipa commented Sep 29, 2022

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/node

SDK Version

7.14.0

Framework Version

@sentry/node@7.14.0

Link to Sentry event

https://sentry.io/organizations/koellisch-gmbh/issues/3591474722/events/c8b2672f4f444952a11d7e012d9302bd/

RangeError: Invalid time value
  ?, in Date.toISOString
  File "C:\home\site\wwwroot\node_modules\@sentry\node\cjs\integrations\context.js", line 177, col 64, in getDeviceContext
  File "C:\home\site\wwwroot\node_modules\@sentry\node\cjs\integrations\context.js", line 95, col 25, in Context._getContexts
  File "C:\home\site\wwwroot\node_modules\@sentry\node\cjs\integrations\context.js", line 51, col 46, in Context.addContext

Steps to Reproduce

  1. prerequisites: node v16.15.1, azure function js runtime 4.x (supports node 14.x - 16.x)
  2. setup Azure functions (with typescript)
  3. cause error/exception

Expected Result

exception to be submitted

Actual Result

event id is being generated but the exception is not being submitted

might be related to the pr from #5512

@Venipa
Copy link
Author

Venipa commented Sep 29, 2022

temporary fix is to rollback to v6 (6.19.7)

@Venipa Venipa changed the title 7.14.0 causes invalid range error (process.uptime) 7.x causes invalid range error (process.uptime) Sep 30, 2022
@Lms24
Copy link
Member

Lms24 commented Sep 30, 2022

Hi @Venipa, thanks for writing in and reporting!

I took a quick look at the event and I think you're right with #5512 causing this. Might be related to this line:

device.boot_time = new Date(Date.now() - os.uptime() * 1000).toISOString();

Perhaps os.uptime() is undefined or returns undefined in azure functions? I tried it out in Node and got exactly the same error when replacing os.uptime with undefined.

I'll open up a quick bugfix PR to get you back to v7.

@Lms24 Lms24 self-assigned this Sep 30, 2022
Lms24 added a commit that referenced this issue Sep 30, 2022
…id (#5859)

When creating the `DeviceContext` in the Node SDK `Context` integration, we calculate the boot time based on `os.uptime`. However, as reported in #5856, `os.uptime` might not always be available or return undefined (I couldn't figure out which of the two cases actually was responsible).

This patch fixes this bug by simply not setting the boot time in case we don't get a valid up time. It also adds two basic tests for this behaviour. I opted to not set the boot time instead of e.g. defaulting to `0` for uptime because IMO this creates a false boot time that probably causes more confusion than not having it.
@Lms24
Copy link
Member

Lms24 commented Sep 30, 2022

I wanted to get to the bottom of this a little more because of @timfish's comment in the bugfix PR.

I therefore created an Azure account and made a little test Http trigger function (Node 16, Windows, Azure Runtime 4). Turns out, os.uptime seems to be the only function we access from os that returns undefined (the function itself is defined). All other os functions we need return valid values.

Excerpt from the execution logs:

2022-09-30T12:40:06Z   [Information]   uptime: undefined
2022-09-30T12:40:06Z   [Information]   arch: ia32
2022-09-30T12:40:06Z   [Information]   totalmem: 3220754432
2022-09-30T12:40:06Z   [Information]   freemem: 1614860288
2022-09-30T12:40:06Z   [Information]   cpus: [
  {
    model: 'Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz',
    speed: 2594,
    times: {
      user: 4724156,
      nice: 0,
      sys: 2849796,
      idle: 321804453,
      irq: 62203
    }
  },
  {
    model: 'Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz',
    speed: 2594,
    times: {
      user: 4391875,
      nice: 0,
      sys: 2726140,
      idle: 322260187,
      irq: 32218
    }
  }
]

@Lms24
Copy link
Member

Lms24 commented Sep 30, 2022

Since it came up: process.uptime() returns a valid number afaict:

2022-09-30T12:51:38Z   [Information]   process.uptime: 30.3910407

@jlev
Copy link

jlev commented Oct 24, 2022

This is causing a lot of false positive events in my Sentry account. I've muted the issue, but I'm worried about blowing through my quota this month. Any timeline on getting a new version of the node package out? Didn't see this change referenced in 7.16.0

@Venipa
Copy link
Author

Venipa commented Oct 24, 2022

temporary fix is to rollback to v6 (6.19.7)

@jlev

@timfish
Copy link
Collaborator

timfish commented Oct 24, 2022

This fix made it into 7.14.1:
https://github.com/getsentry/sentry-javascript/blob/master/CHANGELOG.md#7141

If you are getting issues with any version after this, please open a new issue!

@Lms24
Copy link
Member

Lms24 commented Oct 24, 2022

Hi @jlev the fix to this (#5226) was released in 7.14.1. Is this still a problem in 7.16.0?

@jlev
Copy link

jlev commented Oct 24, 2022

Aha! Missed that point release. Thanks for the quick response, everyone.

@joelcollyer
Copy link

We just started to run into a very similar error with @sentry/node v7.49.0 and @sentry/tracing v7.49.0 in our Express API.

Without any code changes, Sentry suddenly stopped logging data (around Apr 14, 2023), and this issue started to appear in all environments:
image

Based on a recommendation from Sentry Support, we added new Sentry.Integrations.Context({ device: false }), to our Sentry.init and data started to correctly appear again.

Our whole Sentry.init now looks like this (at time of writing):

Sentry.init({
  environment,
  dsn: process.env.SENTRY_DSN,
  integrations: [
    new Sentry.Integrations.Context({ device: false }),
    new Sentry.Integrations.Http({ tracing: true }),
    new Tracing.Integrations.Express({ app })
  ],
  tracesSampleRate: 1.0
});
app.use(Sentry.Handlers.requestHandler());
app.use(Sentry.Handlers.tracingHandler());

@Lms24
Copy link
Member

Lms24 commented May 22, 2023

Hi @joelcollyer, would you mind opening a new issue? I'm not sure how related this is to the original issue, given that you're experiencing this in Express and the original issue was in Azur functions. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants