Surface the underlying error when school onboarding fails

app/services/school_onboarding_service.rb

@@ -14,16 +14,5 @@ def onboard(token:)
 
       ProfileApiClient.create_school(token:, id: school.id, code: school.code)
     end
-  rescue ProfileApiClient::UnauthorizedError
-    # Do not log noise to sentry.
-    # TODO: consider returning a separate error here to distinguish from other errors and return 401 from the API, not 422
-    Rails.logger.warn { "Failed to onboard school #{@school.id}: user is unauthorized" }
-    false
-  rescue StandardError => e
-    Sentry.capture_exception(e)
-    Rails.logger.error { "Failed to onboard school #{@school.id}: #{e.message}" }
-    false
-  else
-    true
   end
 end

lib/concepts/school/operations/create.rb

@@ -10,21 +10,28 @@ def call(school_params:, creator_id:, token:)
         School.transaction do
           response[:school].save!
 
-          onboarded = SchoolOnboardingService.new(response[:school]).onboard(token:)
-          raise 'School onboarding failed' unless onboarded
+          SchoolOnboardingService.new(response[:school]).onboard(token:)
         end
 
         response
+      rescue ProfileApiClient::UnauthorizedError => e
+        # Do not log noise to sentry. The Profile API is only available to verified users over 13.
+        # TODO: consider returning a separate error here to distinguish from other errors and return 401 from the API, not 422
+        Rails.logger.warn { "Failed to onboard school #{response[:school].id}: user is unauthorized" }
+        failure(response, e)
       rescue StandardError => e
         Sentry.capture_exception(e)
-        response[:error] = response[:school].errors.presence || [e.message]
-        response[:error_types] = response[:school].errors.details
-
-        response
+        failure(response, e)
       end
 
       private
 
+      def failure(response, error)
+        response[:error] = response[:school].errors.presence || [error.message]
+        response[:error_types] = response[:school].errors.details
+        response
+      end
+
       def build_school(school_params)
         School.new(school_params)
       end

spec/concepts/school/create_spec.rb

@@ -95,4 +95,53 @@
       expect(onboarding_service).to have_received(:onboard).with(token:)
     end
   end
+
+  context 'when onboarding fails' do
+    let(:error) { ProfileApiClient::Error.new('Profile API is unavailable') }
+
+    before do
+      allow(Sentry).to receive(:capture_exception)
+      allow(ProfileApiClient).to receive(:create_school).and_raise(error)
+    end
+
+    it 'does not create a school' do
+      expect { described_class.call(school_params:, creator_id:, token:) }.not_to change(School, :count)
+    end
+
+    it 'returns a failed operation response' do
+      response = described_class.call(school_params:, creator_id:, token:)
+      expect(response.failure?).to be(true)
+    end
+
+    it 'sends the underlying error to Sentry rather than a generic error' do
+      described_class.call(school_params:, creator_id:, token:)
+      expect(Sentry).to have_received(:capture_exception).with(error)
+    end
+
+    it 'returns the underlying error message in the operation response' do
+      response = described_class.call(school_params:, creator_id:, token:)
+      expect(response[:error]).to eq([error.message])
+    end
+  end
+
+  context 'when onboarding fails because the user is unauthorized in Profile' do
+    before do
+      allow(Sentry).to receive(:capture_exception)
+      allow(ProfileApiClient).to receive(:create_school).and_raise(ProfileApiClient::UnauthorizedError)
+    end
+
+    it 'does not create a school' do
+      expect { described_class.call(school_params:, creator_id:, token:) }.not_to change(School, :count)
+    end
+
+    it 'returns a failed operation response' do
+      response = described_class.call(school_params:, creator_id:, token:)
+      expect(response.failure?).to be(true)
+    end
+
+    it 'does not capture the error in Sentry' do
+      described_class.call(school_params:, creator_id:, token:)
+      expect(Sentry).not_to have_received(:capture_exception)
+    end
+  end
 end

spec/services/school_onboarding_service_spec.rb

@@ -28,10 +28,6 @@
         service.onboard(token:)
         expect(ProfileApiClient).to have_received(:create_school).with(token:, id: school.id, code: school.code)
       end
-
-      it 'returns true' do
-        expect(service.onboard(token:)).to be(true)
-      end
     end
 
     describe 'when the school cannot be created in Profile API' do
@@ -40,43 +36,37 @@
       end
 
       it 'does not create owner role' do
-        service.onboard(token:)
+        suppress(RuntimeError) { service.onboard(token:) }
         expect(school_creator).not_to be_school_owner(school)
       end
 
       it 'does not create teacher role' do
-        service.onboard(token:)
+        suppress(RuntimeError) { service.onboard(token:) }
         expect(school_creator).not_to be_school_teacher(school)
       end
 
-      it 'returns false' do
-        expect(service.onboard(token:)).to be(false)
+      it 'raises the underlying error' do
+        expect { service.onboard(token:) }.to raise_error(RuntimeError)
       end
     end
 
     describe 'when Profile API returns unauthorized' do
       before do
         allow(ProfileApiClient).to receive(:create_school).and_raise(ProfileApiClient::UnauthorizedError)
-        allow(Sentry).to receive(:capture_exception)
       end
 
       it 'does not create owner role' do
-        service.onboard(token:)
+        suppress(ProfileApiClient::UnauthorizedError) { service.onboard(token:) }
         expect(school_creator).not_to be_school_owner(school)
       end
 
       it 'does not create teacher role' do
-        service.onboard(token:)
+        suppress(ProfileApiClient::UnauthorizedError) { service.onboard(token:) }
         expect(school_creator).not_to be_school_teacher(school)
       end
 
-      it 'does not capture the error in Sentry' do
-        service.onboard(token:)
-        expect(Sentry).not_to have_received(:capture_exception)
-      end
-
-      it 'returns false' do
-        expect(service.onboard(token:)).to be(false)
+      it 'raises the underlying error' do
+        expect { service.onboard(token:) }.to raise_error(ProfileApiClient::UnauthorizedError)
       end
     end
 
@@ -88,22 +78,22 @@
       end
 
       it 'does not create owner role' do
-        service.onboard(token:)
+        suppress(ActiveRecord::RecordInvalid) { service.onboard(token:) }
         expect(school_creator).not_to be_school_owner(school)
       end
 
       it 'does not create teacher role' do
-        service.onboard(token:)
+        suppress(ActiveRecord::RecordInvalid) { service.onboard(token:) }
         expect(school_creator).not_to be_school_teacher(school)
       end
 
       it 'does not create school in Profile API' do
-        service.onboard(token:)
+        suppress(ActiveRecord::RecordInvalid) { service.onboard(token:) }
         expect(ProfileApiClient).not_to have_received(:create_school)
       end
 
-      it 'returns false' do
-        expect(service.onboard(token:)).to be(false)
+      it 'raises the underlying error' do
+        expect { service.onboard(token:) }.to raise_error(ActiveRecord::RecordInvalid)
       end
     end
   end