From baa2b7fb46aaa42a0e7d953276df3965968f733f Mon Sep 17 00:00:00 2001
From: Tom de Bruijn <tom@tomdebruijn.com>
Date: Fri, 16 Jul 2021 12:23:24 +0200
Subject: [PATCH] Only send 2 MiB of log data in diagnose report (#415)

Don't send too much log file data. We previously decided upon 2 MiB of
data for the appsignal.log in the Ruby gem, so apply that in the Node.js
integration too.

I wrote the reading of this data in a sync way, with `Sync` functions
from the `fs` module. I could not get it to work with promises and async
without rewriting the entire script to be async, which seemed like a lot
of work for this change.

This sync approach is less safe I think, because we need to close the
open file manually. To make it more safe I've added the file closing to
the `finally` statement that triggers in the try-block regardless if an
error occurs in the try-block. The finally-block is like an ensure-block
in Ruby.

Closes #413
---
 ...signal-log-file-size-in-diagnose-report.md |  5 +++
 .../nodejs/src/__tests__/diagnose.test.ts     | 28 ++++++++++++---
 packages/nodejs/src/diagnose.ts               | 36 ++++++++++++++++++-
 3 files changed, 64 insertions(+), 5 deletions(-)
 create mode 100644 packages/nodejs/.changesets/limit-appsignal-log-file-size-in-diagnose-report.md

diff --git a/packages/nodejs/.changesets/limit-appsignal-log-file-size-in-diagnose-report.md b/packages/nodejs/.changesets/limit-appsignal-log-file-size-in-diagnose-report.md
new file mode 100644
index 00000000..415025fa
--- /dev/null
+++ b/packages/nodejs/.changesets/limit-appsignal-log-file-size-in-diagnose-report.md
@@ -0,0 +1,5 @@
+---
+bump: "patch"
+---
+
+Limit the `appsignal.log` file size in diagnose report. It will only send the last 2MiB of the file. This prevents the diagnose report from sending too much data that it gets rejected by the server.
diff --git a/packages/nodejs/src/__tests__/diagnose.test.ts b/packages/nodejs/src/__tests__/diagnose.test.ts
index f8057df9..ceda26b0 100644
--- a/packages/nodejs/src/__tests__/diagnose.test.ts
+++ b/packages/nodejs/src/__tests__/diagnose.test.ts
@@ -1,3 +1,4 @@
+import fs from "fs"
 import { DiagnoseTool } from "../diagnose"
 import { VERSION, AGENT_VERSION } from "../version"
 
@@ -31,10 +32,29 @@ describe("DiagnoseTool", () => {
     expect(tool.generate().paths.log_dir_path.path).toEqual("/tmp")
   })
 
-  it("returns the appsignal.log path", () => {
-    expect(tool.generate().paths["appsignal.log"].path).toEqual(
-      "/tmp/appsignal.log"
-    )
+  describe("appsignal.log path", () => {
+    it("returns the appsignal.log path", () => {
+      expect(tool.generate().paths["appsignal.log"].path).toEqual(
+        "/tmp/appsignal.log"
+      )
+    })
+
+    it("returns all of the file if file is less than 2MiB in size", () => {
+      const fileSize = 1.9 * 1024 * 1024 // Write 1.9 MiB of content
+      const content = ".".repeat(fileSize)
+      fs.writeFileSync("/tmp/appsignal.log", content, { flag: "w" })
+
+      const contentArray = tool.generate().paths["appsignal.log"].content
+      expect(contentArray?.join("\n").length).toBeCloseTo(fileSize, 0)
+    })
+
+    it("returns maximum 2MiB of content", () => {
+      const content = ".".repeat(2.1 * 1024 * 1024) // Write 2.1 MiB of content
+      fs.writeFileSync("/tmp/appsignal.log", content, { flag: "w" })
+
+      const contentArray = tool.generate().paths["appsignal.log"].content
+      expect(contentArray?.join("\n").length).toEqual(2 * 1024 * 1024)
+    })
   })
 
   describe("when to log path is configured as a full path", () => {
diff --git a/packages/nodejs/src/diagnose.ts b/packages/nodejs/src/diagnose.ts
index 6cb79a8d..567e42c5 100644
--- a/packages/nodejs/src/diagnose.ts
+++ b/packages/nodejs/src/diagnose.ts
@@ -183,18 +183,52 @@ function getPathType(stats: fs.Stats) {
   }
 }
 
+const BYTES_TO_READ_FOR_FILES = 2 * 1024 * 1024 // 2 Mebibytes
+
 /**
  * Attempts to read a UTF-8 from `path`, and either returns the result
  * as a string, or an empty string on error
  */
 function safeReadFromPath(path: string): string {
   try {
-    return fs.readFileSync(path, "utf8")
+    return readBytesFromPath(path, BYTES_TO_READ_FOR_FILES)
   } catch (_) {
     return ""
   }
 }
 
+function readBytesFromPath(path: string, bytesToRead: number): string {
+  let fd
+  try {
+    const { readLength, startPosition } = readFileOptions(path, bytesToRead)
+    fd = fs.openSync(path, "r")
+    const buffer = Buffer.alloc(readLength)
+    fs.readSync(fd, buffer, 0, readLength, startPosition)
+    return buffer.toString("utf8")
+  } finally {
+    if (fd) {
+      fs.closeSync(fd)
+    }
+  }
+}
+
+function readFileOptions(path: string, bytesToRead: number) {
+  const stats = fs.statSync(path)
+  const fileSize = stats.size
+  if (fileSize < bytesToRead) {
+    return {
+      readLength: fileSize,
+      startPosition: 0
+    }
+  } else {
+    const startPosition = fileSize - bytesToRead
+    return {
+      readLength: bytesToRead,
+      startPosition
+    }
+  }
+}
+
 /**
  * the following lines are borrowed from https://github.com/sindresorhus/is-docker/
  * thanks sindre! <3