Repository: denoland/deno
Pull request: fix(ext/node): `os.setPriority` and `os.getPriority` compatibility

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](https://github.com/nodejs/node/blob/v24.2.0/test/parallel/test-os-process-priority.js) to pass.

Node.js 24.5.0:
```bash
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:
```bash
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:
```bash
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']
}
```

Changed files:
filename: ext/node/ops/os/priority.rs
patch: @@ -21,8 +21,6 @@ mod impl_ {
   use libc::PRIO_PROCESS;
   use libc::id_t;
 
-  const PRIORITY_HIGH: i32 = -14;
-
   // Ref: https://github.com/libuv/libuv/blob/55376b044b74db40772e8a6e24d67a8673998e02/src/unix/core.c#L1533-L1547
   pub fn get_priority(pid: u32) -> Result<i32, super::PriorityError> {
     set_errno(Errno(0));
@@ -31,7 +29,7 @@ mod impl_ {
       unsafe { libc::getpriority(PRIO_PROCESS, pid as id_t) },
       errno(),
     ) {
-      (-1, Errno(0)) => Ok(PRIORITY_HIGH),
+      (-1, Errno(0)) => Ok(-1),
       (-1, _) => Err(std::io::Error::last_os_error().into()),
       (priority, _) => Ok(priority),
     }
@@ -66,6 +64,7 @@ mod impl_ {
   use winapi::um::winbase::NORMAL_PRIORITY_CLASS;
   use winapi::um::winbase::REALTIME_PRIORITY_CLASS;
   use winapi::um::winnt::PROCESS_QUERY_LIMITED_INFORMATION;
+  use winapi::um::winnt::PROCESS_SET_INFORMATION;
 
   // Taken from: https://github.com/libuv/libuv/blob/a877ca2435134ef86315326ef4ef0c16bdbabf17/include/uv.h#L1318-L1323
   const PRIORITY_LOW: i32 = 19;
@@ -113,7 +112,7 @@ mod impl_ {
       let handle = if pid == 0 {
         GetCurrentProcess()
       } else {
-        OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid as DWORD)
+        OpenProcess(PROCESS_SET_INFORMATION, FALSE, pid as DWORD)
       };
       if handle == NULL {
         Err(std::io::Error::last_os_error().into())

filename: ext/node/polyfills/internal/errors.ts
patch: @@ -2681,6 +2681,31 @@ export function denoErrorToNodeError(e: Error, ctx: UvExceptionContext) {
   return ex;
 }
 
+export const denoErrorToNodeSystemError = hideStackFrames((
+  e: Error,
+  syscall: string,
+): Error => {
+  const osErrno = extractOsErrorNumberFromErrorMessage(e);
+  if (typeof osErrno === "undefined") {
+    return e;
+  }
+
+  const uvErrno = mapSysErrnoToUvErrno(osErrno);
+  const { 0: code, 1: message } = uvErrmapGet(uvErrno) || uvUnmappedError;
+  const ctx: NodeSystemErrorCtx = {
+    errno: uvErrno,
+    code,
+    message,
+    syscall,
+  };
+
+  return new NodeSystemError(
+    "ERR_SYSTEM_ERROR",
+    ctx,
+    "A system error occurred",
+  );
+});
+
 function extractOsErrorNumberFromErrorMessage(e: unknown): number | undefined {
   const match = ObjectPrototypeIsPrototypeOf(ErrorPrototype, e)
     ? StringPrototypeMatch(e.message, new SafeRegExp(/\(os error (\d+)\)/))
@@ -3088,6 +3113,7 @@ export default {
   codes,
   connResetException,
   denoErrorToNodeError,
+  denoErrorToNodeSystemError,
   dnsException,
   errnoException,
   errorMap,

filename: ext/node/polyfills/os.ts
patch: @@ -31,13 +31,14 @@ import {
   op_node_os_user_info,
 } from "ext:core/ops";
 
-import { validateIntegerRange } from "ext:deno_node/_utils.ts";
 import process from "node:process";
 import { isWindows } from "ext:deno_node/_util/os.ts";
 import { os } from "ext:deno_node/internal_binding/constants.ts";
 import { osUptime } from "ext:deno_os/30_os.js";
 import { Buffer } from "ext:deno_node/internal/buffer.mjs";
 import { primordials } from "ext:core/mod.js";
+import { validateInt32 } from "ext:deno_node/internal/validators.mjs";
+import { denoErrorToNodeSystemError } from "ext:deno_node/internal/errors.ts";
 
 const {
   ObjectDefineProperties,
@@ -176,8 +177,12 @@ export function freemem(): number {
 
 /** Not yet implemented */
 export function getPriority(pid = 0): number {
-  validateIntegerRange(pid, "pid");
-  return op_node_os_get_priority(pid);
+  validateInt32(pid, "pid");
+  try {
+    return op_node_os_get_priority(pid);
+  } catch (error) {
+    throw denoErrorToNodeSystemError(error as Error, "uv_os_getpriority");
+  }
 }
 
 /** Returns the string path of the current user's home directory. */
@@ -266,10 +271,15 @@ export function setPriority(pid: number, priority?: number) {
     priority = pid;
     pid = 0;
   }
-  validateIntegerRange(pid, "pid");
-  validateIntegerRange(priority, "priority", -20, 19);
 
-  op_node_os_set_priority(pid, priority);
+  validateInt32(pid, "pid");
+  validateInt32(priority, "priority", -20, 19);
+
+  try {
+    op_node_os_set_priority(pid, priority);
+  } catch (error) {
+    throw denoErrorToNodeSystemError(error as Error, "uv_os_setpriority");
+  }
 }
 
 /** Returns the operating system's default directory for temporary files as a string. */

filename: tests/node_compat/config.toml
patch: @@ -807,6 +807,7 @@
 "parallel/test-nodeeventtarget.js" = {}
 "parallel/test-os-eol.js" = {}
 "parallel/test-os-homedir-no-envvar.js" = {}
+"parallel/test-os-process-priority.js" = {}
 "parallel/test-os-userinfo-handles-getter-errors.js" = {}
 "parallel/test-os.js" = {}
 "parallel/test-outgoing-message-destroy.js" = {}

filename: tests/unit_node/os_test.ts
patch: @@ -99,14 +99,14 @@ Deno.test({
         os.getPriority(3.15);
       },
       Error,
-      "pid must be 'an integer'",
+      'The value of "pid" is out of range. It must be an integer',
     );
     assertThrows(
       () => {
         os.getPriority(9999999999);
       },
       Error,
-      "must be >= -2147483648 && <= 2147483647",
+      'The value of "pid" is out of range. It must be >= -2147483648 && <= 2147483647',
     );
   },
 });
@@ -119,14 +119,14 @@ Deno.test({
         os.setPriority(3.15, 0);
       },
       Error,
-      "pid must be 'an integer'",
+      `The value of "pid" is out of range. It must be an integer`,
     );
     assertThrows(
       () => {
         os.setPriority(9999999999, 0);
       },
       Error,
-      "pid must be >= -2147483648 && <= 2147483647",
+      'The value of "pid" is out of range. It must be >= -2147483648 && <= 2147483647',
     );
   },
 });
@@ -139,28 +139,28 @@ Deno.test({
         os.setPriority(0, 3.15);
       },
       Error,
-      "priority must be 'an integer'",
+      'The value of "priority" is out of range. It must be an integer',
     );
     assertThrows(
       () => {
         os.setPriority(0, -21);
       },
       Error,
-      "priority must be >= -20 && <= 19",
+      'The value of "priority" is out of range. It must be >= -20 && <= 19',
     );
     assertThrows(
       () => {
         os.setPriority(0, 20);
       },
       Error,
-      "priority must be >= -20 && <= 19",
+      'The value of "priority" is out of range. It must be >= -20 && <= 19',
     );
     assertThrows(
       () => {
         os.setPriority(0, 9999999999);
       },
       Error,
-      "priority must be >= -20 && <= 19",
+      'The value of "priority" is out of range. It must be >= -20 && <= 19',
     );
   },
 });
@@ -174,28 +174,28 @@ Deno.test({
         os.setPriority(3.15);
       },
       Error,
-      "priority must be 'an integer'",
+      'The value of "priority" is out of range. It must be an integer',
     );
     assertThrows(
       () => {
         os.setPriority(-21);
       },
       Error,
-      "priority must be >= -20 && <= 19",
+      'The value of "priority" is out of range. It must be >= -20 && <= 19',
     );
     assertThrows(
       () => {
         os.setPriority(20);
       },
       Error,
-      "priority must be >= -20 && <= 19",
+      'The value of "priority" is out of range. It must be >= -20 && <= 19',
     );
     assertThrows(
       () => {
         os.setPriority(9999999999);
       },
       Error,
-      "priority must be >= -20 && <= 19",
+      'The value of "priority" is out of range. It must be >= -20 && <= 19',
     );
   },
 });
@@ -294,6 +294,9 @@ Deno.test({
 Deno.test({
   name:
     "os.setPriority() throw os permission denied error & os.getPriority() doesn't",
+  // Windows allows setting process priority without privilege escalation up to `PRIORITY_HIGH`.
+  // Setting to `PRIORITY_HIGHEST` will be silently reduced to `PRIORITY_HIGH`.
+  ignore: Deno.build.os === "windows",
   async fn() {
     const child = new Deno.Command(Deno.execPath(), {
       args: ["eval", "while (true) { console.log('foo') }"],
@@ -307,14 +310,39 @@ Deno.test({
           throw err;
         }
       },
-      Deno.errors.PermissionDenied,
+      Error,
+      "uv_os_setpriority returned EACCES (permission denied)",
     );
     os.getPriority(child.pid);
     child.kill();
     await child.status;
   },
 });
 
+Deno.test({
+  name: "os.setPriority() works on Windows",
+  ignore: Deno.build.os !== "windows",
+  async fn() {
+    const child = new Deno.Command(Deno.execPath(), {
+      args: ["eval", "while (true) { console.log('foo') }"],
+    }).spawn();
+
+    const originalPriority = os.getPriority(child.pid);
+    assertNotEquals(originalPriority, os.constants.priority.PRIORITY_HIGH);
+    os.setPriority(child.pid, os.constants.priority.PRIORITY_HIGH);
+
+    assertEquals(
+      os.getPriority(child.pid),
+      os.constants.priority.PRIORITY_HIGH,
+    );
+
+    os.setPriority(child.pid, originalPriority);
+    assertEquals(os.getPriority(child.pid), originalPriority);
+    child.kill();
+    await child.status;
+  },
+});
+
 // Gets the diff in log_10 scale
 function diffLog10(a: number, b: number): number {
   return Math.abs(Math.log10(a) - Math.log10(b));