Revert "Fix user login timeout error from bugsnag"

Author: compwron

app/controllers/users/passwords_controller.rb

@@ -47,12 +47,6 @@ def send_password
 
   def send_password_reset_mail
     @reset_token = @resource.send_reset_password_instructions # generate a reset token and call devise mailer
-  rescue Net::ReadTimeout, Net::OpenTimeout => e
-    # Log the error but don't expose it to the user for security reasons
-    Rails.logger.error("Password reset email failed to send: #{e.class} - #{e.message}")
-    Bugsnag.notify(e) if defined?(Bugsnag)
-    # Still generate a token so SMS can potentially work
-    @reset_token = @resource.generate_password_reset_token
   end
 
   def send_password_reset_sms
@@ -95,7 +89,6 @@ def empty_fields_error
   end
 
   def invalid_phone_number_error(error_message)
-    @resource ||= resource
     @resource.errors.add(:phone_number, error_message)
 
     false

config/environments/production.rb

@@ -15,9 +15,7 @@
     user_name: ENV["SENDINBLUE_EMAIL"],
     password: ENV["SENDINBLUE_PASSWORD"],
     authentication: "login",
-    enable_starttls_auto: true,
-    open_timeout: 5,  # Timeout for opening connection (seconds)
-    read_timeout: 5   # Timeout for reading response (seconds)
+    enable_starttls_auto: true
   }
   # Code is not reloaded between requests.
   config.enable_reloading = false

spec/requests/users/passwords_spec.rb

@@ -4,17 +4,17 @@
   let!(:org) { create(:casa_org) }
   let!(:user) { create(:user, phone_number: "+12222222222", casa_org: org) }
 
-  let!(:twilio_service_double) { instance_double(TwilioService) }
+  let!(:twillio_service_double) { instance_double(TwilioService) }
   let!(:short_url_service_double) { instance_double(ShortUrlService) }
 
   before do
     allow(TwilioService).to(
       receive(:new).with(
         org
-      ).and_return(twilio_service_double)
+      ).and_return(twillio_service_double)
     )
 
-    allow(twilio_service_double).to receive(:send_sms)
+    allow(twillio_service_double).to receive(:send_sms)
 
     allow(ShortUrlService).to receive(:new).and_return(short_url_service_double)
 
@@ -37,14 +37,14 @@
 
       it "sends a password reset SMS to existing user" do
         request
-        expect(twilio_service_double).to have_received(:send_sms).once.with(
+        expect(twillio_service_double).to have_received(:send_sms).once.with(
           {From: org.twilio_phone_number, Body: a_string_matching("reset_url"), To: user.phone_number}
         )
       end
 
       it "sends a password reset email to existing user" do
-        expect { request }.to change { ActionMailer::Base.deliveries.count }.by(1)
-        expect(ActionMailer::Base.deliveries.last.to).to include(user.email)
+        expect_any_instance_of(User).to receive(:send_reset_password_instructions).once
+        request
       end
 
       it { is_expected.to redirect_to(user_session_url) }
@@ -60,13 +60,13 @@
         let(:params) { {user: {email: user.email, phone_number: ""}} }
 
         it "sends a password reset email to existing user" do
-          expect { request }.to change { ActionMailer::Base.deliveries.count }.by(1)
-          expect(ActionMailer::Base.deliveries.last.to).to include(user.email)
+          expect_any_instance_of(User).to receive(:send_reset_password_instructions).once
+          request
         end
 
         it "does not send sms with reset password" do
           request
-          expect(twilio_service_double).not_to have_received(:send_sms)
+          expect(twillio_service_double).not_to have_received(:send_sms)
         end
       end
 
@@ -75,13 +75,14 @@
 
         it "sends a password reset SMS to existing user" do
           request
-          expect(twilio_service_double).to have_received(:send_sms).once.with(
+          expect(twillio_service_double).to have_received(:send_sms).once.with(
             {From: org.twilio_phone_number, Body: a_string_matching("reset_url"), To: user.phone_number}
           )
         end
 
         it "does not send email with reset password" do
-          expect { request }.not_to change { ActionMailer::Base.deliveries.count }
+          expect_any_instance_of(User).not_to receive(:send_reset_password_instructions)
+          request
         end
       end
     end
@@ -95,35 +96,17 @@
       end
     end
 
-    context "with wrong parameters (non-existent user)" do
+    context "with wrong parameters" do
       let(:params) { {user: {phone_number: "13333333333"}} }
 
-      it "does not reveal if user exists (security)" do
+      it "sets errors correctly" do
         request
         expect(flash[:notice]).to(
           eq("If the account exists you will receive an email or SMS with instructions on how to reset your password in a few minutes.")
         )
       end
     end
 
-    context "with invalid phone number format" do
-      let(:params) { {user: {email: "", phone_number: "1234"}} }
-
-      it "shows phone number validation error" do
-        request
-        expect(request.parsed_body.to_html).to include("phone_number")
-      end
-    end
-
-    context "with non-numeric phone number" do
-      let(:params) { {user: {email: "", phone_number: "abc"}} }
-
-      it "shows phone number validation error" do
-        request
-        expect(request.parsed_body.to_html).to include("phone_number")
-      end
-    end
-
     context "when twilio is disabled" do
       let(:params) { {user: {email: user.email, phone_number: user.phone_number}} }
 
@@ -132,87 +115,12 @@
       end
 
       it "does not send an sms, only an email" do
-        expect { request }.to change { ActionMailer::Base.deliveries.count }.by(1)
-        expect(flash[:notice]).to(
-          eq("If the account exists you will receive an email or SMS with instructions on how to reset your password in a few minutes.")
-        )
-      end
-    end
-
-    context "when email sending times out with Net::ReadTimeout" do
-      let(:params) { {user: {email: user.email, phone_number: user.phone_number}} }
-
-      before do
-        allow(user).to receive(:send_reset_password_instructions).and_raise(Net::ReadTimeout)
-        allow(User).to receive(:find_by).and_return(user)
-      end
-
-      it "handles the timeout gracefully and still shows success message" do
-        expect(Rails.logger).to receive(:error).with(/Password reset email failed to send/)
-        request
-        expect(flash[:notice]).to(
-          eq("If the account exists you will receive an email or SMS with instructions on how to reset your password in a few minutes.")
-        )
-      end
-
-      it "does not crash the request" do
-        expect { request }.not_to raise_error
-        expect(response).to redirect_to(user_session_url)
-      end
-
-      it "notifies Bugsnag of the error" do
-        expect(Bugsnag).to receive(:notify).with(instance_of(Net::ReadTimeout))
-        request
-      end
-
-      it "generates a fallback token for SMS to use" do
-        expect(user).to receive(:generate_password_reset_token).and_call_original
-        request
-      end
-
-      it "still sends SMS with the fallback token" do
-        request
-        expect(twilio_service_double).to have_received(:send_sms).once
-      end
-    end
-
-    context "when email sending times out with Net::OpenTimeout" do
-      let(:params) { {user: {email: user.email, phone_number: ""}} }
-
-      before do
-        allow(user).to receive(:send_reset_password_instructions).and_raise(Net::OpenTimeout)
-        allow(User).to receive(:find_by).and_return(user)
-      end
-
-      it "handles the timeout gracefully and still shows success message" do
-        expect(Rails.logger).to receive(:error).with(/Password reset email failed to send/)
+        expect_any_instance_of(User).to receive(:send_reset_password_instructions).once
         request
         expect(flash[:notice]).to(
           eq("If the account exists you will receive an email or SMS with instructions on how to reset your password in a few minutes.")
         )
       end
-
-      it "does not crash the request" do
-        expect { request }.not_to raise_error
-        expect(response).to redirect_to(user_session_url)
-      end
-
-      it "notifies Bugsnag of the error" do
-        expect(Bugsnag).to receive(:notify).with(instance_of(Net::OpenTimeout))
-        request
-      end
-    end
-
-    context "when SMS sending fails" do
-      let(:params) { {user: {email: "", phone_number: user.phone_number}} }
-
-      before do
-        allow(twilio_service_double).to receive(:send_sms).and_raise(Twilio::REST::TwilioError.new("Service unavailable"))
-      end
-
-      it "raises the error (no rescue in controller)" do
-        expect { request }.to raise_error(Twilio::REST::TwilioError)
-      end
     end
   end
 
@@ -233,58 +141,12 @@
       }
     end
 
-    subject(:request) { put user_password_path, params: params }
+    subject(:submit_reset) { put user_password_path, params: params }
 
-    context "with valid token and password" do
-      it "successfully resets the password" do
-        request
-        expect(response).to redirect_to(new_user_session_path)
-        expect(flash[:notice]).to eq("Your password has been changed successfully.")
-      end
-
-      it "allows user to sign in with new password" do
-        request
-        user.reload
-        expect(user.valid_password?("newpassword123!")).to be true
-      end
-    end
-
-    context "with password mismatch" do
-      let(:params) do
-        {
-          user: {
-            reset_password_token: token,
-            password: "newpassword123!",
-            password_confirmation: "differentpassword123!"
-          }
-        }
-      end
-
-      it "does not reset the password" do
-        old_password_digest = user.encrypted_password
-        request
-        user.reload
-        expect(user.encrypted_password).to eq(old_password_digest)
-      end
-    end
-
-    context "with password too short" do
-      let(:params) do
-        {
-          user: {
-            reset_password_token: token,
-            password: "abc",
-            password_confirmation: "abc"
-          }
-        }
-      end
-
-      it "does not reset the password" do
-        old_password_digest = user.encrypted_password
-        request
-        user.reload
-        expect(user.encrypted_password).to eq(old_password_digest)
-      end
+    it "successfully resets the password" do
+      submit_reset
+      expect(response).to redirect_to(new_user_session_path)
+      expect(flash[:notice]).to eq("Your password has been changed successfully.")
     end
   end
 end