fix: handle and retry DNS errors (#5326)

* fix: handle DNS error codes as ErrnoException

DNS error codes are not included in the OS error constants
(os.constants) / https://nodejs.org/docs/latest-v18.x/api/os.html#error-constants

This commit adds DNS error codes to the list of handled errno errors.

Fixes #5217

* test: isErrnoException

* test: remove unnecessary assertions

* fix(lint): remove unused import

core/src/analytics/analytics.ts

@@ -23,7 +23,7 @@ import { Profile } from "../util/profiling"
 import { ModuleConfig } from "../config/module"
 import { UserResult } from "@garden-io/platform-api-types"
 import { uuidv4 } from "../util/random"
-import { GardenError, NodeJSErrnoErrorCodes, StackTraceMetadata } from "../exceptions"
+import { GardenError, NodeJSErrnoException, StackTraceMetadata } from "../exceptions"
 import { ActionConfigMap } from "../actions/types"
 import { actionKinds } from "../actions/types"
 import { getResultErrorProperties } from "./helpers"
@@ -163,7 +163,7 @@ export type AnalyticsGardenErrorDetail = {
   /**
    * If this error was caused by an underlying NodeJSErrnoException, this will be the code.
    */
-  code?: NodeJSErrnoErrorCodes
+  code?: NodeJSErrnoException["code"]
 
   stackTrace?: StackTraceMetadata
 }

core/src/exceptions.ts

@@ -14,19 +14,48 @@ import stripAnsi from "strip-ansi"
 import { Cycle } from "./graph/common"
 import indentString from "indent-string"
 import { constants } from "os"
+import dns from "node:dns"
+
+// Unfortunately, NodeJS does not provide a list of all error codes, so we have to maintain this list manually.
+// See https://nodejs.org/docs/latest-v18.x/api/dns.html#error-codes
+const dnsErrorCodes = [
+  dns.NODATA,
+  dns.FORMERR,
+  dns.SERVFAIL,
+  dns.NOTFOUND,
+  dns.NOTIMP,
+  dns.REFUSED,
+  dns.BADQUERY,
+  dns.BADNAME,
+  dns.BADFAMILY,
+  dns.BADRESP,
+  dns.CONNREFUSED,
+  dns.TIMEOUT,
+  dns.EOF,
+  dns.FILE,
+  dns.NOMEM,
+  dns.DESTRUCTION,
+  dns.BADSTR,
+  dns.BADFLAGS,
+  dns.NONAME,
+  dns.BADHINTS,
+  dns.NOTINITIALIZED,
+  dns.LOADIPHLPAPI,
+  dns.ADDRGETNETWORKPARAMS,
+  dns.CANCELLED,
+]
 
 // See https://nodejs.org/api/os.html#error-constants
-type NodeJSErrnoErrors = typeof constants.errno
-export type NodeJSErrnoErrorCodes = keyof NodeJSErrnoErrors
+const errnoErrors = Object.keys(constants.errno)
 
-const errnoErrorCodeSet = new Set(Object.keys(constants.errno))
+const errnoErrorCodeSet = new Set([errnoErrors, dnsErrorCodes].flat())
 
 /**
  * NodeJS native errors with a code property.
  */
 export type NodeJSErrnoException = NodeJS.ErrnoException & {
-  code: NodeJSErrnoErrorCodes
-  errno: number
+  // Unfortunately we can't make this type more concrete, as DNS error codes are not known at typescript compile time.
+  code: string
 }
 
 export type EAddrInUseException = NodeJSErrnoException & {
@@ -37,7 +66,7 @@ export type EAddrInUseException = NodeJSErrnoException & {
 }
 
 export function isErrnoException(err: any): err is NodeJSErrnoException {
-  return typeof err.code === "string" && typeof err.errno === "number" && errnoErrorCodeSet.has(err.code)
+  return typeof err.code === "string" && errnoErrorCodeSet.has(err.code)
 }
 
 export function isEAddrInUseException(err: any): err is EAddrInUseException {
@@ -64,7 +93,7 @@ export interface GardenErrorParams {
    */
   readonly taskType?: string
 
-  readonly code?: NodeJSErrnoErrorCodes
+  readonly code?: NodeJSErrnoException["code"]
 }
 
 export abstract class GardenError extends Error {
@@ -81,7 +110,7 @@ export abstract class GardenError extends Error {
   /**
    * If there was an underlying NodeJSErrnoException, the error code
    */
-  public code?: NodeJSErrnoErrorCodes
+  public code?: NodeJSErrnoException["code"]
 
   public wrappedErrors?: GardenError[]
 
@@ -354,7 +383,7 @@ export class InternalError extends GardenError {
   static wrapError(error: Error | string | any, prefix?: string): InternalError {
     let message: string | undefined
     let stack: string | undefined
-    let code: NodeJSErrnoErrorCodes | undefined
+    let code: NodeJSErrnoException["code"] | undefined
 
     if (isErrnoException(error)) {
       message = error.message

core/src/plugins/kubernetes/retry.ts

@@ -15,8 +15,9 @@ import { dedent, deline } from "../../util/string"
 import { LogLevel } from "../../logger/logger"
 import { KubernetesError } from "./api"
 import requestErrors = require("request-promise/errors")
-import { InternalError, NodeJSErrnoErrorCodes, isErrnoException } from "../../exceptions"
+import { InternalError, NodeJSErrnoException, isErrnoException } from "../../exceptions"
 import { ErrorEvent } from "ws"
+import dns from "node:dns"
 
 /**
  * The flag {@code forceRetry} can be used to avoid {@link shouldRetry} helper call in case if the error code
@@ -93,7 +94,7 @@ export function toKubernetesError(err: unknown, context: string): KubernetesErro
   let response: KubernetesClientHttpError["response"] | undefined
   let body: any | undefined
   let responseStatusCode: number | undefined
-  let code: NodeJSErrnoErrorCodes | undefined
+  let code: NodeJSErrnoException["code"] | undefined
 
   if (err instanceof KubernetesClientHttpError) {
     errorType = "HttpError"
@@ -159,7 +160,7 @@ export function toKubernetesError(err: unknown, context: string): KubernetesErro
 export function shouldRetry(error: unknown, context: string): boolean {
   const err = toKubernetesError(error, context)
 
-  if (err.code === "ECONNRESET") {
+  if (err.code && errorCodesForRetry.includes(err.code)) {
     return true
   }
 
@@ -185,6 +186,8 @@ export const statusCodesForRetry: number[] = [
   524, // A Timeout Occurred
 ]
 
+const errorCodesForRetry: NodeJSErrnoException["code"][] = ["ECONNRESET", dns.NOTFOUND]
+
 const errorMessageRegexesForRetry = [
   /ETIMEDOUT/,
   /ENOTFOUND/,

core/test/unit/src/exceptions.ts

@@ -14,9 +14,54 @@ import {
   RuntimeError,
   StackTraceMetadata,
   getStackTraceMetadata,
+  isErrnoException,
 } from "../../../src/exceptions"
 import dedent from "dedent"
 import { testFlags } from "../../../src/util/util"
+import { readFile } from "fs-extra"
+import { resolve4 } from "dns/promises"
+import dns from "node:dns"
+
+describe("isErrnoException", async () => {
+  it("should return true for file not found errors", async () => {
+    let err: unknown
+
+    try {
+      await readFile("non-existent-file")
+      expect.fail("should have thrown")
+    } catch (e) {
+      err = e
+    }
+
+    if (isErrnoException(err)) {
+      expect(err.code).to.equal("ENOENT")
+    } else {
+      expect.fail("should have been an NodeJSErrnoException")
+    }
+  })
+
+  it("should return true for DNS ENOTFOUND errors", async () => {
+    let err: unknown
+
+    try {
+      await resolve4("non-existent-hostname")
+      expect.fail("should have thrown")
+    } catch (e) {
+      err = e
+    }
+
+    if (isErrnoException(err)) {
+      expect(err.code).to.equal(dns.NOTFOUND)
+    } else {
+      expect.fail("should have been an NodeJSErrnoException")
+    }
+  })
+
+  it("should return false for other errors", () => {
+    const err = new Error("test exception")
+    expect(isErrnoException(err)).to.be.false
+  })
+})
 
 describe("GardenError", () => {
   // helper to avoid dealing with changing line numbers