fix: use find_by! to return 404 for invalid tokens

[mroderick]

Summary

Converts silent nil returns from find_by to find_by! in before_action callbacks, causing Rails to return a proper 404 response instead of a 500 error when invalid tokens are accessed.

Problem

In production, users visiting /invitations/:token with invalid tokens were seeing:

NoMethodError: undefined method 'member' for nil

This happened because:

1. InvitationController#show uses @invitation.member on line 5
2. The @invitation variable was set by a before_action using find_by(token: ...)
3. find_by returns nil when no record matches (instead of raising an exception)
4. Calling .member on nil causes NoMethodError

Root Cause Analysis

The issue was identified in app/controllers/concerns/workshop_invitation_concerns.rb:25-27:

def set_invitation
  @invitation = WorkshopInvitation.find_by(token: token)  # Returns nil if not found
end

This pattern existed in 9 locations across the codebase, using find_by instead of find_by! for token/slug lookups in before_action callbacks.

Solution

Changed find_by to find_by! in all 9 locations. The find_by! method raises ActiveRecord::RecordNotFound when no record is found, which Rails automatically converts to a 404 response.

Files Changed

Priority	File	Line
High	app/controllers/concerns/workshop_invitation_concerns.rb	26
High	app/controllers/admin/meeting_invitations_controller.rb	37
High	app/controllers/admin/invitations_controller.rb	76
Medium	app/controllers/admin/meetings_controller.rb	57
Medium	app/controllers/events_controller.rb	71
Medium	app/controllers/admin/events_controller.rb	70
Low	app/controllers/invitations_controller.rb	80
Low	app/controllers/contact_preferences_controller.rb	11
Low	app/controllers/feedback_controller.rb	22

Testing

All controller tests pass. Note: Some pre-existing test failures exist in the codebase (related to 'twitter' attribute on Member model), but these are unrelated to this fix.

Impact

• Before: Invalid tokens caused 500 errors (NoMethodError)
• After: Invalid tokens return 404 Not Found

This is more user-friendly and follows REST conventions - an invalid token represents a resource that doesn't exist, not a server error.

Audit Notes

A broader audit of the codebase was performed to identify similar patterns. All find_by calls in before_action callbacks that could cause nil-related crashes have been addressed.

[KimberleyCook]

🎉

[till]

IMHO: keep it without ! and instead handle the error to display a transparent error message?

class CodebarError < StandardError
  def initialize(message)
    super(message)
  end

  # ...
end

For the base controller:

rescue_from CodebarError do |_exception|
  ...
end

Suggested change

	@original_event = Event.find_by!(slug: params[:id])
	e = Event.find_by(slug: params[:id])
	if e.nil?
	raise CodebarError.new("could not find event")
	@original_event = e

[olleolleolle]

I like the conventional ! to raise standard ActiveRecord errors which have a built-in 404 handling.

[till]

But doesn't that make for rather bad UX? Assumes whoever reads it, knows what token we mean, etc.. I mean, I won't 👎🏼 this. Just don't think this is super great.

[olleolleolle]

Spelling this out, too: A good thing about this change is that we will get fewer logged errors to investigate - 404s won't show up in exception-tracking services.

Having said all that, yes, we could possibly sometime make failed authentication return something even more useful and legible to the client that offered a wrong token.

But this is a start.

[mroderick]

Let's get this one merged, so we can stop with the 5xx errors and have 4xx errors instead.

I've created a discussion to explore making the UX around errors better: #2572