fix(ext/node): os.setPriority and os.getPriority compatibility

[Tango992]

Closes #30405

Changes in this PR:

• Fixes os.setPriority() on Windows.
• Validates the function parameters correctly and constructs NodeSystemError on op call error.
• Allows parallel/test-os-process-priority.js to pass.

Node.js 24.5.0:

node test-os.ts
SystemError [ERR_SYSTEM_ERROR]: A system error occurred: uv_os_setpriority returned EACCES (permission denied)
    at Module.setPriority (node:os:263:11)
    ... redacted ...
  code: 'ERR_SYSTEM_ERROR',
  info: {
    errno: -13,
    code: 'EACCES',
    message: 'permission denied',
    syscall: 'uv_os_setpriority'
  },
  errno: [Getter/Setter: -13],
  syscall: [Getter/Setter: 'uv_os_setpriority']
}

Deno 2.4.3:

deno -A test-os.ts                                                          
PermissionDenied: Permission denied (os error 13)
    at Module.setPriority (node:os:160:3)
    ... redacted ...
  name: "PermissionDenied",
  code: "EACCES"
}

This PR:

deno -A test-os.ts
SystemError: A system error occurred: uv_os_setpriority returned EACCES (permission denied)
    at __node_internal_captureLargerStackTrace (ext:deno_node/internal/errors.ts:156:5)
    ... redacted ...
  code: 'ERR_SYSTEM_ERROR',
  info: {
    errno: -13,
    code: 'EACCES',
    message: 'permission denied',
    syscall: 'uv_os_setpriority'
  },
  errno: [Getter/Setter: -13],
  syscall: [Getter/Setter: 'uv_os_setpriority']
}

[Tango992]

For unknown reasons, the test seems to always fail on macos-aarch64 on CI, even though it works fine on my device with the same architecture.

These are the two jobs that failed with the same rejection:

• https://github.com/denoland/deno/actions/runs/16925294235/job/47959686407
• https://github.com/denoland/deno/actions/runs/16938294495/job/48000242470

It passes on linux CI which made me think it was macos-aarch64 specific. My speculation is our macos-aarch64 CI doesn't allow some processes to set priority levels by themself.

[bartlomieju]

Pining @willnewby for some help

[bartlomieju]

Also - this might not be CI related, but rather a problem in implementation of op_node_os_set_priority

[bartlomieju]

ChatGPT says that it might need root access (though it should return error if that's not the case)

[Tango992]

Also - this might not be CI related, but rather a problem in implementation of op_node_os_set_priority

After digging into the op, I think it's because of our incorrect implementation of op_node_os_get_priority (not the op_node_os_set_priority):

deno/ext/node/ops/os/priority.rs

Lines 27 to 38 in 85ec26f

	pub fn get_priority(pid: u32) -> Result<i32, super::PriorityError> {
	set_errno(Errno(0));
	match (
	// SAFETY: libc::getpriority is unsafe
	unsafe { libc::getpriority(PRIO_PROCESS, pid as id_t) },
	errno(),
	) {
	(-1, Errno(0)) => Ok(PRIORITY_HIGH),
	(-1, _) => Err(std::io::Error::last_os_error().into()),
	(priority, _) => Ok(priority),
	}
	}

Here's libuv's implementation:
https://github.com/libuv/libuv/blob/55376b044b74db40772e8a6e24d67a8673998e02/src/unix/core.c#L1533-L1547

I think this line is incorrect and explains why on macos CI, it always returns -14 (the value of PRIORITY_HIGH):

deno/ext/node/ops/os/priority.rs

Line 34 in 85ec26f

(-1, Errno(0)) => Ok(PRIORITY_HIGH),

Some reference from https://man7.org/linux/man-pages/man3/errno.3.html :

For some system calls and library functions (e.g.,
getpriority(2)), -1 is a valid return on success. In such cases,
a successful return can be distinguished from an error return by
setting errno to zero before the call, and then, if the call
returns a status that indicates that an error may have occurred,
checking to see if errno has a nonzero value.

[Tango992]

Though documentation on Windows is quite scarce, I got this from here: https://github.com/nodejs/node/blob/591ba692bfe30408e6a67397e7d18bfa1b9c3561/test/parallel/test-os-process-priority.js#L131-L132

The reason why previously this test passes on Windows is because os.setPriority() always throw no matter what. See #30405

[bartlomieju]

That sounds good, but does that mean we are not able to run some of the Node tests in our CI? Maybe we should add a way to ignore certain OSes in config.toml?

[Tango992]

We don't need to ignore certain OSes from our node compat tests, because the Windows' os.setPriority() issue is already addressed in this PR. I'll update the PR message so it covers that.

But having a way to ignore certain OSes from the node compat test sounds good to have.

[bartlomieju]

Opened #30439

[bartlomieju]

LGTM!