Check if logPath fallback is writable

When the configured logPath is not writable, the integration falls back
on the system tmp dir. This commit checks if that directory is also
writable by the app process or not. We've seen this happen before in the
Ruby integration, so let's add the same check here.

This change means that the `logFilePath` getter can sometimes return an
`undefined` value. The integration will then not set the
`_APPSIGNAL_LOG_FILE_PATH` env variable, as it would point to a path
that can't be written to. In this case the agent will try to find a log
directory on its own, which will probably not succeed either and it will
shut down.

packages/nodejs/.changesets/fix-log-writable-check.md

@@ -0,0 +1,5 @@
+---
+bump: "patch"
+---
+
+Check if the fallback log directory can be written to and print a warning if no log can be written there.

packages/nodejs/src/__tests__/config.test.ts

@@ -184,7 +184,11 @@ describe("Configuration", () => {
           const fsAccessSpy = jest
             .spyOn(fs, "accessSync")
             .mockImplementation(path => {
-              throw "Error"
+              if (path === "/foo_dir") {
+                throw "Error"
+              } else {
+                return true
+              }
             })
           const warnMock = jest
             .spyOn(console, "warn")
@@ -197,6 +201,24 @@ describe("Configuration", () => {
           )
           expect(config.logFilePath).toEqual("/tmp/appsignal.log")
         })
+
+        it("return undefined if the system tmp dir can't be written to", () => {
+          const fsAccessSpy = jest
+            .spyOn(fs, "accessSync")
+            .mockImplementation(() => {
+              throw "Error"
+            })
+          const warnMock = jest
+            .spyOn(console, "warn")
+            .mockImplementation(() => {})
+
+          config = new Configuration({ logPath: "/foo_dir" })
+
+          expect(warnMock).toHaveBeenLastCalledWith(
+            `Unable to log to '/foo_dir' or '/tmp' fallback. Please check the permissions of these directories.`
+          )
+          expect(config.logFilePath).toBeUndefined()
+        })
       })
     })
 

packages/nodejs/src/config.ts

@@ -56,7 +56,7 @@ export class Configuration {
     return (this.data.pushApiKey || "").trim() !== ""
   }
 
-  public get logFilePath(): string {
+  public get logFilePath(): string | undefined {
     const filename = "appsignal.log"
     let logPath = this.data["logPath"]
     if (logPath && path.extname(logPath) != "") {
@@ -71,12 +71,22 @@ export class Configuration {
       return path.join(logPath, filename)
     } else {
       const tmpDir = this._tmpdir()
-      if (logPath) {
+      if (isWritable(tmpDir)) {
+        if (logPath) {
+          console.warn(
+            `Unable to log to '${logPath}'. Logging to '${tmpDir}' instead. Please check the permissions of the 'logPath' directory.`
+          )
+        }
+        return path.join(tmpDir, filename)
+      } else {
+        let configuredPath = ""
+        if (logPath) {
+          configuredPath = `'${logPath}' or `
+        }
         console.warn(
-          `Unable to log to '${logPath}'. Logging to '${tmpDir}' instead. Please check the permissions of the 'logPath' directory.`
+          `Unable to log to ${configuredPath}'${tmpDir}' fallback. Please check the permissions of these directories.`
         )
       }
-      return path.join(tmpDir, filename)
     }
   }
 
@@ -176,7 +186,10 @@ export class Configuration {
    */
   private writePrivateConfig(config: { [key: string]: any }) {
     this.writePrivateConstants()
-    process.env["_APPSIGNAL_LOG_FILE_PATH"] = this.logFilePath
+    const logFilePath = this.logFilePath
+    if (logFilePath) {
+      process.env["_APPSIGNAL_LOG_FILE_PATH"] = logFilePath
+    }
 
     // write to a "private" environment variable if it exists in the
     // config structure

packages/nodejs/src/diagnose.ts

@@ -149,33 +149,42 @@ export class DiagnoseTool {
         path: process.cwd()
       },
       log_dir_path: {
-        path: path.dirname(logFilePath)
+        path: logFilePath ? path.dirname(logFilePath) : ""
       },
       "appsignal.log": {
-        path: logFilePath,
-        content: safeReadFromPath(logFilePath).trimEnd().split("\n")
+        path: logFilePath || "",
+        content: logFilePath
+          ? safeReadFromPath(logFilePath).trimEnd().split("\n")
+          : []
       }
     }
 
     Object.entries(pathsToCheck).forEach(([key, data]) => {
       const { path } = data
 
-      try {
-        const stats = fs.statSync(path)
-        const { mode, gid, uid } = stats
-
-        paths[key] = {
-          ...data,
-          exists: true,
-          mode: mode.toString(8),
-          ownership: {
-            gid,
-            uid
-          },
-          type: getPathType(stats),
-          writable: isWritable(path)
+      if (fs.existsSync(path)) {
+        try {
+          let stats = fs.statSync(path)
+          const { mode, gid, uid } = stats
+
+          paths[key] = {
+            ...data,
+            exists: true,
+            mode: mode.toString(8),
+            ownership: {
+              gid,
+              uid
+            },
+            type: getPathType(stats),
+            writable: isWritable(path)
+          }
+        } catch (_) {
+          paths[key] = {
+            ...data,
+            exists: true
+          }
         }
-      } catch (_) {
+      } else {
         paths[key] = {
           ...data,
           exists: false