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 missing node/os functionality #17850

Closed
20 tasks done
Tracked by #18836
cknight opened this issue Jan 26, 2020 · 13 comments · Fixed by #19370
Closed
20 tasks done
Tracked by #18836

Support missing node/os functionality #17850

cknight opened this issue Jan 26, 2020 · 13 comments · Fixed by #19370
Labels
help wanted community help requested node compat

Comments

@cknight
Copy link
Contributor

cknight commented Jan 26, 2020

I am looking into the Node polyfill for 'os' ( #3403 ). The following Node functionality does not seem to be available in Deno:

The 'os' polyfill cannot be fully completed until the above items are implemented in Deno.

@cknight cknight changed the title Support reporting of CPU information Support missing OS functionality Jan 26, 2020
@ecyrbe
Copy link
Contributor

ecyrbe commented Feb 29, 2020

i'm currently working on this

@ecyrbe
Copy link
Contributor

ecyrbe commented Mar 1, 2020

CC @ry can i use the crate systemstat to implement :

  • freemem()
  • totalmem()
  • cpus()
  • uptime()

and more...

it is under more activity than sys_info and made entirely in rust. unlike what the documentation suggests, the author added windows support for all of these.

I really don't wan't to make my own /proc/stat /proc/cpuinfo parser and windows compatibility module if someone already made one.

your call.

@ecyrbe
Copy link
Contributor

ecyrbe commented Mar 1, 2020

Or if we are not confident enough, use a libuv binding : https://docs.rs/libuv-sys2/1.34.4/libuv_sys2/
but this would be a huge depedency that i'm not sure we want to pay such a price.

@ry
Copy link
Member

ry commented Mar 2, 2020

We certainly are not adding libuv (despite it being our product) it overlaps too much with Tokio.

I will look into sys_info vs systemstat.

@ecyrbe
Copy link
Contributor

ecyrbe commented Mar 8, 2020

@ry did you have time to investigate ?

@ry
Copy link
Member

ry commented Mar 8, 2020

@ecyrbe I haven't really, but systemstat seems quite reasonable. It has CI for windows unlike sys_info (which has been broken on windows for the last two releases). If anyone felt inclined to switch us from sys-info to systemstat, that would be welcome.

I haven't checked if systemstat actually does all the things that sys-info does tho.

@vladimyr
Copy link

vladimyr commented Jun 2, 2020

CC @ry can i use the crate systemstat to implement :

  • freemem()
  • totalmem()

Unfortunately, you won't get really far with it. It doesn't support memory info on macOS: https://github.com/myfreeweb/systemstat/blob/a7c54aa/src/platform/macos.rs#L58-L60

@juliangruber

This comment has been minimized.

@bartlomieju bartlomieju transferred this issue from denoland/deno Oct 19, 2021
@bartlomieju bartlomieju changed the title Support missing OS functionality Support missing node/os functionality Oct 19, 2021
@denoland denoland deleted a comment from stale bot Nov 24, 2021
@denoland denoland deleted a comment from stale bot Nov 24, 2021
@denoland denoland deleted a comment from stale bot Nov 24, 2021
bnoordhuis referenced this issue in denoland/std Nov 24, 2021
bnoordhuis referenced this issue in bnoordhuis/deno Dec 2, 2021
Add an op to list the network interfaces on the system.

Prep work for denoland#8137 and `os.networkInterfaces()` Node compat in std.

Refs denoland/deno_std#1436.
bnoordhuis referenced this issue in bnoordhuis/deno Dec 13, 2021
Add an op to list the network interfaces on the system.

Prep work for denoland#8137 and `os.networkInterfaces()` Node compat in std.

Refs denoland/deno_std#1436.
bnoordhuis referenced this issue in bnoordhuis/deno Dec 22, 2021
Add an op to list the network interfaces on the system.

Prep work for denoland#8137 and `os.networkInterfaces()` Node compat in std.

Refs denoland/deno_std#1436.
bnoordhuis referenced this issue Dec 22, 2021
Add an op to list the network interfaces on the system.

Prep work for #8137 and `os.networkInterfaces()` Node compat in std.

Refs denoland/deno_std#1436.
@joakimunge
Copy link

Looks like networkInterfaces was merged into Deno a few days ago in #13475

Should networkInterfaces here then be disregarded?

@cknight
Copy link
Contributor Author

cknight commented Jan 31, 2022

Looks like networkInterfaces was merged into Deno a few days ago in denoland/deno#13475

Should networkInterfaces here then be disregarded?

Updated, thanks for the reminder

@khrj
Copy link
Contributor

khrj commented Nov 24, 2022

os.version() and os.machine() are also missing now

@khrj khrj mentioned this issue Nov 24, 2022
@GJZwiers
Copy link
Contributor

GJZwiers commented Dec 17, 2022

npm:@sentry/node fails on missing os.uptime for versions 7.12.0+:

import * as Sentry from "npm:@sentry/node@7.12.0";

Sentry.init({
  dsn: "<my-sentry-dsn>",
  tracesSampleRate: 1.0,
});

async function testEvent() {
  try {
    throw new Error("Nope.");
  } catch (e) {
    Sentry.captureException(e);
    await Sentry.flush();
  }
}

await testEvent();

Error shown in Sentry UI:

Error: Not implemented: See https://github.com/denoland/deno/issues/17850
  File "/std@0.168.0/node/_utils.ts", line 23, col 9, in notImplemented
  File "/std@0.168.0/node/os.ts", line 310, col 3, in Object.uptime
  File "/C:/Users/GJZwiers/AppData/Local/deno/npm/registry.npmjs.org/@sentry/node/7.12.0/cjs/integrations/context.js", line 169, col 47, in getDeviceContext
  File "/C:/Users/GJZwiers/AppData/Local/deno/npm/registry.npmjs.org/@sentry/node/7.12.0/cjs/integrations/context.js", line 87, col 25, in Context._getContexts
  File "/C:/Users/GJZwiers/AppData/Local/deno/npm/registry.npmjs.org/@sentry/node/7.12.0/cjs/integrations/context.js", line 51, col 66, in Context.addContext

Edit: Fixed in 1.29.2

@waldyrious
Copy link
Contributor

Can someone (@crowlKats?) mark the "getPriority" and "userInfo" checkboxes in the opening comment above as completed, and link both to #19370?

Also, shouldn't this test (and possibly this one too) be uncommented? /CC @kt3k

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted community help requested node compat
Projects
None yet
Development

Successfully merging a pull request may close this issue.