From 2e93182b6ae83b16fe9885558cd8f0bfce6a9a5f Mon Sep 17 00:00:00 2001
From: Tom de Bruijn <tom@tomdebruijn.com>
Date: Tue, 12 Dec 2023 12:04:48 +0100
Subject: [PATCH] Fix diagnose file read error on illegal seeks (#1016)

When an app configured the log_file_path to something like `/dev/stdout`
we can read from that source, but it will return an illegal seek error.
Catch any such IO errors with SystemCallError and store the error.

Requires a server update on the viewer to display the error.
---
 .../fix-error-in-diagnose-report-path-read.md |  6 ++++++
 lib/appsignal/cli/diagnose.rb                 |  5 +++++
 lib/appsignal/cli/diagnose/paths.rb           | 12 +++++++----
 spec/lib/appsignal/cli/diagnose/utils_spec.rb | 11 ++++++++++
 spec/lib/appsignal/cli/diagnose_spec.rb       | 20 +++++++++++++++++++
 5 files changed, 50 insertions(+), 4 deletions(-)
 create mode 100644 .changesets/fix-error-in-diagnose-report-path-read.md

diff --git a/.changesets/fix-error-in-diagnose-report-path-read.md b/.changesets/fix-error-in-diagnose-report-path-read.md
new file mode 100644
index 000000000..b8b8fa2b1
--- /dev/null
+++ b/.changesets/fix-error-in-diagnose-report-path-read.md
@@ -0,0 +1,6 @@
+---
+bump: "patch"
+type: "fix"
+---
+
+Fix an error in the diagnose report when reading a file's contents results in an "Invalid seek" error. This could happen when the log path is configured to `/dev/stdout`, which is not supported.
diff --git a/lib/appsignal/cli/diagnose.rb b/lib/appsignal/cli/diagnose.rb
index 0fd57ba14..4ac318fa4 100644
--- a/lib/appsignal/cli/diagnose.rb
+++ b/lib/appsignal/cli/diagnose.rb
@@ -631,6 +631,11 @@ def print_path_details(name, path)
           else
             print_empty_line
           end
+
+          return unless path.key?(:read_error)
+
+          puts "    Read error: #{path[:read_error]}"
+          print_empty_line
         end
 
         def print_empty_line
diff --git a/lib/appsignal/cli/diagnose/paths.rb b/lib/appsignal/cli/diagnose/paths.rb
index f85e4d718..989fed6e2 100644
--- a/lib/appsignal/cli/diagnose/paths.rb
+++ b/lib/appsignal/cli/diagnose/paths.rb
@@ -71,10 +71,14 @@ def path_stat(path)
               :group => Utils.group_for_gid(path_gid)
             }
             if info[:type] == "file"
-              info[:content] = Utils.read_file_content(
-                path,
-                BYTES_TO_READ_FOR_FILES
-              ).split("\n")
+              begin
+                info[:content] = Utils.read_file_content(
+                  path,
+                  BYTES_TO_READ_FOR_FILES
+                ).split("\n")
+              rescue => error
+                info[:read_error] = "#{error.class}: #{error.message}"
+              end
             end
           end
         end
diff --git a/spec/lib/appsignal/cli/diagnose/utils_spec.rb b/spec/lib/appsignal/cli/diagnose/utils_spec.rb
index bdeb71a97..e2e37eea9 100644
--- a/spec/lib/appsignal/cli/diagnose/utils_spec.rb
+++ b/spec/lib/appsignal/cli/diagnose/utils_spec.rb
@@ -71,5 +71,16 @@
         is_expected.to eq(file_contents)
       end
     end
+
+    context "when reading the file raises an illegal seek error" do
+      let(:file_contents) { "line 1\n" }
+      before do
+        expect(File).to receive(:binread).and_raise(Errno::ESPIPE)
+      end
+
+      it "returns the error as the content" do
+        expect { subject }.to raise_error(Errno::ESPIPE)
+      end
+    end
   end
 end
diff --git a/spec/lib/appsignal/cli/diagnose_spec.rb b/spec/lib/appsignal/cli/diagnose_spec.rb
index 799e2dbbb..4ccbe54bf 100644
--- a/spec/lib/appsignal/cli/diagnose_spec.rb
+++ b/spec/lib/appsignal/cli/diagnose_spec.rb
@@ -1424,6 +1424,26 @@ def expect_config_to_be_printed(config)
             )
           end
         end
+
+        context "when reading the file returns a illegal seek error" do
+          before do
+            File.write(file_path, "Some content")
+            allow(File).to receive(:binread).and_call_original
+            expect(File).to receive(:binread).with(file_path, anything,
+              anything).and_raise(Errno::ESPIPE)
+            run
+          end
+
+          it "outputs file does not exists" do
+            expect(output).to include %(Read error: Errno::ESPIPE: Illegal seek)
+          end
+
+          it "transmits file data in report" do
+            expect(received_report["paths"][filename]).to include(
+              "read_error" => "Errno::ESPIPE: Illegal seek"
+            )
+          end
+        end
       end
 
       describe "mkmf.log" do