Skip to content

Commit

Permalink
Fix Pathname to String conversion error (#1310)
Browse files Browse the repository at this point in the history
In Rails apps the app root path is a Pathname instance. We can't use it
the same way as a String when using it in `.start_with?` when comparing
backtrace lines for error causes in
`Transaction#first_formatted_backtrace_line`.

Fix this by casting the root_path always to a String in the Config
initializer so we're always sure it's a String and can use it as such.
  • Loading branch information
tombruijn authored Sep 28, 2024
1 parent ff98ed6 commit b767f26
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: fix
---

Fix 'no implicit conversion of Pathname into String' error when parsing backtrace lines of error causes in Rails apps.
2 changes: 1 addition & 1 deletion lib/appsignal/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def initialize(
root_path,
env
)
@root_path = root_path
@root_path = root_path.to_s
@config_file_error = false
@config_file = config_file
@valid = false
Expand Down
9 changes: 9 additions & 0 deletions spec/lib/appsignal/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,15 @@ def on_load
end
end

context "when root path is Pathname instance" do
let(:config) { described_class.new(Pathname.new("/path"), "production") }

it "converts it to a String" do
expect(config.root_path).to eq("/path")
expect(config.root_path).to be_instance_of(String)
end
end

context "without config file" do
let(:config) { described_class.new(tmp_dir, "production") }

Expand Down
2 changes: 1 addition & 1 deletion spec/lib/appsignal/integrations/railtie_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def initialize_railtie(event)
it "sets the Rails app path as root_path" do
initialize_railtie(event)

expect(Appsignal.config.root_path).to eq(Pathname.new(rails_project_fixture_path))
expect(Appsignal.config.root_path).to eq(rails_project_fixture_path)
end

it "loads the Rails app name in the initial config" do
Expand Down

2 comments on commit b767f26

@jeffbax
Copy link

@jeffbax jeffbax commented on b767f26 Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one really got us the past few days :(

@tombruijn
Copy link
Member Author

@tombruijn tombruijn commented on b767f26 Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeffbax how do you mean? Did this fix it for you or did it cause another problem?

Please sign in to comment.