chore: requested changes

Author: vladfrangu

doc/api/os.md

@@ -516,9 +516,9 @@ added: REPLACEME
 
 * `fd` {integer} The file descriptor number to try and guess the type of.
 
-* Returns: {string}
+* Returns: {string|null}
 
-Returns the type of the file descriptor passed in, or `'INVALID'` if the provided file descriptor
+Returns the type of the file descriptor passed in, or `null` if the provided file descriptor
 is invalid.
 
 Currently, the following types for a file descriptor can be returned:
@@ -529,7 +529,6 @@ Currently, the following types for a file descriptor can be returned:
 * `'FILE'`
 * `'PIPE'`
 * `'UNKNOWN'`
-* `'INVALID'`
 
 ## OS constants
 

lib/internal/util.js

@@ -52,6 +52,7 @@ const {
 
 const {
   codes: {
+    ERR_INVALID_FD,
     ERR_NO_CRYPTO,
     ERR_NO_TYPESCRIPT,
     ERR_UNKNOWN_SIGNAL,
@@ -899,11 +900,11 @@ function getCIDR(address, netmask, family) {
 }
 
 const handleTypes = ['TCP', 'TTY', 'UDP', 'FILE', 'PIPE', 'UNKNOWN'];
-setOwnProperty(handleTypes, -1, 'INVALID');
+setOwnProperty(handleTypes, -1, null);
 
 function guessHandleType(fd) {
-  if (!NumberIsInteger(fd)) {
-    return 'INVALID';
+  if (fd >> 0 !== fd || fd < 0) {
+    throw new ERR_INVALID_FD(fd);
   }
 
   const type = _guessHandleType(fd);

src/node_util.cc

@@ -208,7 +208,7 @@ static uint32_t GetUVHandleTypeCode(const uv_handle_type type) {
       return 5;
     default:
       // For an unhandled handle type, we want to return `UNKNOWN` instead of
-      // `INVALID` since the type is "known" by UV, just not exposed further to
+      // `null` since the type is "known" by UV, just not exposed further to
       // JS land
       return 5;
   }
@@ -221,7 +221,7 @@ static void GuessHandleType(const FunctionCallbackInfo<Value>& args) {
   if (!args[0]->Int32Value(context).To(&fd)) return;
 
   // If the provided file descriptor is not valid, we return `-1`, which in JS
-  // land will be marked as "INVALID"
+  // land will be marked as null
   if (fd < 0) [[unlikely]] {
     args.GetReturnValue().Set(-1);
     return;

test/pseudo-tty/test-os-guessFileDescriptorType.js

@@ -1,17 +1,19 @@
 'use strict';
 
 require('../common');
-const { strictEqual } = require('assert');
+const { strictEqual, throws } = require('assert');
 const { guessFileDescriptorType } = require('os');
 
 strictEqual(guessFileDescriptorType(0), 'TTY', 'stdin reported to not be a tty, but it is');
 strictEqual(guessFileDescriptorType(1), 'TTY', 'stdout reported to not be a tty, but it is');
 strictEqual(guessFileDescriptorType(2), 'TTY', 'stderr reported to not be a tty, but it is');
 
-strictEqual(guessFileDescriptorType(-1), 'INVALID', '-1 reported to be a tty, but it is not');
-strictEqual(guessFileDescriptorType(55555), 'UNKNOWN', '55555 reported to be a tty, but it is not');
-strictEqual(guessFileDescriptorType(2 ** 31), 'INVALID', '2^31 reported to be a tty, but it is not');
-strictEqual(guessFileDescriptorType(1.1), 'INVALID', '1.1 reported to be a tty, but it is not');
-strictEqual(guessFileDescriptorType('1'), 'INVALID', '\'1\' reported to be a tty, but it is not');
-strictEqual(guessFileDescriptorType({}), 'INVALID', '{} reported to be a tty, but it is not');
-strictEqual(guessFileDescriptorType(() => {}), 'INVALID', '() => {} reported to be a tty, but it is not');
+strictEqual(guessFileDescriptorType(55555), 'UNKNOWN', '55555 reported to be a handle, but it is not');
+strictEqual(guessFileDescriptorType(2 ** 31 - 1), 'UNKNOWN', '2^31-1 reported to be a handle, but it is not');
+
+throws(() => guessFileDescriptorType(-1), /"fd" must be a positive integer/, '-1 reported to be a handle, but it is not');
+throws(() => guessFileDescriptorType(1.1), /"fd" must be a positive integer/, '1.1 reported to be a handle, but it is not');
+throws(() => guessFileDescriptorType('1'), /"fd" must be a positive integer/, '\'1\' reported to be a tty, but it is not');
+throws(() => guessFileDescriptorType({}), /"fd" must be a positive integer/, '{} reported to be a tty, but it is not');
+throws(() => guessFileDescriptorType(() => {}), /"fd" must be a positive integer/, '() => {} reported to be a tty, but it is not');
+throws(() => guessFileDescriptorType(2 ** 31), /"fd" must be a positive integer/, '2^31 reported to be a handle, but it is not (because the fd check rolls over the input to negative of it)');