Skip to content

Read url_helper_name and url_options from what their writers set - #108

Merged
fractaledmind merged 1 commit into
fractaledmind:mainfrom
namolnad:fix-url-config-writers
Oct 16, 2026
Merged

fractaledmind merged 1 commit into
fractaledmind:mainfrom
namolnad:fix-url-config-writers

Conversation

@namolnad

Copy link
Copy Markdown
Contributor

url_helper_name and url_options are declared with mattr_writer, which stores the value in a class variable (@@url_options). Their readers memoize an instance variable (@url_options ||= detect_url_options) and never look at it. So the engine's config.solid_errors.each { SolidErrors.public_send("#{name}=", value) } writes a value that nothing reads, and the email falls back to route detection.

That shows up as an error email with no link whenever detection can't find the route. Two common cases:

  • The engine is mounted under a name that doesn't end in solid_errors, e.g. mount SolidErrors::Engine, at: "errors", as: "admin_errors". Detection falls back to :solid_errors, occurrence_url raises NoMethodError, the helper rescues it, and the link is dropped.
  • The dashboard lives on a different host from ActionMailer's default_url_options, so the link needs url_options: {host: ...}.
config.solid_errors.url_options = {host: "admin.example.com", protocol: "https"}
SolidErrors.url_options # => {} before this change

The fix makes the readers memoize into the same class variables the writers set. Explicit config wins, and detection still fills in when nothing is configured.

Added a mailer test that sets url_options and checks the link. It fails before the change and passes after.

mattr_writer stores the value in a class variable, but the readers memoized
an instance variable, so config.solid_errors.url_options and url_helper_name
were silently ignored and the error email fell back to detection.
@namolnad
namolnad marked this pull request as ready for review October 13, 2026 16:16
@fractaledmind
fractaledmind merged commit ed46600 into fractaledmind:main Oct 16, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants