Skip to content

Commit

Permalink
Fix diagnose file read error on illegal seeks (#1016)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tombruijn authored Dec 12, 2023
1 parent 667c2d0 commit 2e93182
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 4 deletions.
6 changes: 6 additions & 0 deletions .changesets/fix-error-in-diagnose-report-path-read.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions lib/appsignal/cli/diagnose.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions lib/appsignal/cli/diagnose/paths.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions spec/lib/appsignal/cli/diagnose/utils_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 20 additions & 0 deletions spec/lib/appsignal/cli/diagnose_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2e93182

Please sign in to comment.