fix(h1): ignore type of service errors

lib/core/request.js

@@ -173,7 +173,7 @@ class Request {
 
     this.method = method
 
-    this.typeOfService = typeOfService ?? 0
+    this.typeOfService = typeOfService
 
     this.abort = null
 

lib/dispatcher/client-h1.js

@@ -60,6 +60,7 @@ const removeAllListeners = util.removeAllListeners
 const kIdleSocketValidation = Symbol('kIdleSocketValidation')
 const kIdleSocketValidationTimeout = Symbol('kIdleSocketValidationTimeout')
 const kSocketUsed = Symbol('kSocketUsed')
+const kTypeOfService = Symbol('kTypeOfService')
 
 let extractBody
 
@@ -1134,6 +1135,32 @@ function shouldSendContentLength (method) {
   return method !== 'GET' && method !== 'HEAD' && method !== 'OPTIONS' && method !== 'TRACE' && method !== 'CONNECT'
 }
 
+function setTypeOfService (socket, request) {
+  if (typeof socket.setTypeOfService !== 'function') {
+    return
+  }
+
+  const typeOfService = request.typeOfService
+
+  if (typeOfService === undefined) {
+    return
+  }
+
+  const currentTypeOfService = socket[kTypeOfService]
+
+  if (currentTypeOfService === typeOfService) {
+    return
+  }
+
+  try {
+    socket.setTypeOfService(typeOfService)
+    socket[kTypeOfService] = typeOfService
+  } catch {
+    // QoS marking is best-effort. setTypeOfService() can throw synchronously on
+    // some platforms depending on socket state, but that must not abort the request.
+  }
+}
+
 /**
  * @param {import('./client.js')} client
  * @param {import('../core/request.js')} request
@@ -1265,9 +1292,7 @@ function writeH1 (client, request) {
     socket[kBlocking] = true
   }
 
-  if (socket.setTypeOfService) {
-    socket.setTypeOfService(request.typeOfService)
-  }
+  setTypeOfService(socket, request)
 
   let header = `${method} ${path} HTTP/1.1\r\n`
 

test/ip-prioritization.js

@@ -1,12 +1,14 @@
 'use strict'
 
+const assert = require('node:assert')
 const { test, after } = require('node:test')
 const { Client } = require('..')
 const { createServer } = require('node:http')
 const { once } = require('node:events')
+const net = require('node:net')
 
-test('HTTP/1.1 Request Prioritization', async (t) => {
-  let priority = null
+test('HTTP/1.1 Request Prioritization', async () => {
+  const priorities = []
 
   const server = createServer((req, res) => {
     res.end('ok')
@@ -17,36 +19,120 @@ test('HTTP/1.1 Request Prioritization', async (t) => {
 
   const client = new Client(`http://localhost:${server.address().port}`, {
     connect: (opts, cb) => {
-      const socket = require('node:net').connect({
+      const socket = net.connect({
         ...opts,
         host: opts.hostname,
         port: opts.port
       }, () => {
         cb(null, socket)
       })
       socket.setTypeOfService = (p) => {
-        priority = p
+        priorities.push(p)
       }
       return socket
     }
   })
   after(() => client.close())
   after(() => server.close())
 
-  await client.request({
+  const response = await client.request({
     path: '/',
     method: 'GET',
     typeOfService: 42
   })
+  await response.body.text()
 
-  // Check if priority was set
-  if (priority !== 42) {
-    throw new Error(`Expected priority 42, got ${priority}`)
-  }
+  const response2 = await client.request({
+    path: '/',
+    method: 'GET'
+  })
+  await response2.body.text()
+
+  assert.deepStrictEqual(priorities, [42])
+})
+
+// https://github.com/nodejs/undici/issues/5544
+// The default request path should not touch setsockopt on a fresh socket.
+test('HTTP/1.1 Request Prioritization skips default ToS on fresh socket', async () => {
+  let calls = 0
+
+  const server = createServer((req, res) => {
+    res.end('ok')
+  })
+
+  server.listen(0)
+  await once(server, 'listening')
+
+  const client = new Client(`http://localhost:${server.address().port}`, {
+    connect: (opts, cb) => {
+      const socket = net.connect({
+        ...opts,
+        host: opts.hostname,
+        port: opts.port
+      }, () => {
+        cb(null, socket)
+      })
+      socket.setTypeOfService = () => {
+        calls++
+        throw new Error('setTypeOfService EINVAL')
+      }
+      return socket
+    }
+  })
+  after(() => client.close())
+  after(() => server.close())
+
+  const response = await client.request({
+    path: '/',
+    method: 'GET'
+  })
+  await response.body.text()
+
+  assert.strictEqual(calls, 0)
+})
+
+// https://github.com/nodejs/undici/issues/5544
+// setTypeOfService() is best-effort and must not make the request fail.
+test('HTTP/1.1 Request Prioritization ignores setTypeOfService errors', async () => {
+  const priorities = []
+
+  const server = createServer((req, res) => {
+    res.end('ok')
+  })
+
+  server.listen(0)
+  await once(server, 'listening')
+
+  const client = new Client(`http://localhost:${server.address().port}`, {
+    connect: (opts, cb) => {
+      const socket = net.connect({
+        ...opts,
+        host: opts.hostname,
+        port: opts.port
+      }, () => {
+        cb(null, socket)
+      })
+      socket.setTypeOfService = (p) => {
+        priorities.push(p)
+        throw new Error('setTypeOfService EINVAL')
+      }
+      return socket
+    }
+  })
+  after(() => client.close())
+  after(() => server.close())
+
+  const response = await client.request({
+    path: '/',
+    method: 'GET',
+    typeOfService: 42
+  })
+
+  assert.strictEqual(await response.body.text(), 'ok')
+  assert.deepStrictEqual(priorities, [42])
 })
 
 test('HTTP/2 Connection Prioritization', async (t) => {
-  const net = require('node:net')
   const buildConnector = require('../lib/core/connect')
 
   let receivedHints = null

types/dispatcher.d.ts

@@ -111,7 +111,7 @@ declare namespace Dispatcher {
     idempotent?: boolean;
     /** Whether the response is expected to take a long time and would end up blocking the pipeline. When this is set to `true` further pipelining will be avoided on the same connection until headers have been received. Defaults to `method !== 'HEAD'`. */
     blocking?: boolean;
-    /** The IP Type of Service (ToS) value for the request socket. Must be an integer between 0 and 255. Default: `0` */
+    /** The IP Type of Service (ToS) value for the request socket. Must be an integer between 0 and 255. */
     typeOfService?: number | null;
     /** Upgrade the request. Should be used to specify the kind of upgrade i.e. `'Websocket'`. Default: `method === 'CONNECT' || null`. */
     upgrade?: boolean | string | null;