Fix error structure

Knerio · Knerio · commit 87a46c9098a7 · 2025-11-13T19:30:59.000+01:00
diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb
@@ -27,7 +27,12 @@ def current_authentication
     end
 
     def create_error(code, message)
-      { code: code, details: message }
+      ErrorCode.validate_error_code!(code)
+
+      Sagittarius::Graphql::ErrorContainer.new(
+        code,
+        [{ message: message }]
+      )
     end
   end
 end
diff --git a/app/graphql/mutations/users/register.rb b/app/graphql/mutations/users/register.rb
@@ -16,7 +16,9 @@ class Register < BaseMutation
       argument :username, String, required: true, description: 'Username of the user'
 
       def resolve(username:, email:, password:, password_repeat:)
-        return { user: nil, errors: [create_message_error('Invalid password repeat')] } if password != password_repeat
+        if password != password_repeat
+          return { user: nil, errors: [create_error(:invalid_password_repeat, 'Invalid password repeat')] }
+        end
 
         response = ::Users::RegisterService.new(
           username,
diff --git a/app/graphql/types/errors/detailed_error_type.rb b/app/graphql/types/errors/detailed_error_type.rb
@@ -13,7 +13,7 @@ def self.resolve_type(object, _ctx)
         case object
         when ActiveModel::Error
           Types::Errors::ActiveModelErrorType
-        when Sagittarius::Graphql::ErrorMessageContainer
+        when Hash
           Types::Errors::MessageErrorType
         else
           raise 'Unsupported DetailedErrorType'
diff --git a/app/graphql/types/errors/error_code_type.rb b/app/graphql/types/errors/error_code_type.rb
diff --git a/app/graphql/types/errors/error_type.rb b/app/graphql/types/errors/error_type.rb
@@ -8,8 +8,9 @@ class ErrorType < Types::BaseObject
       # rubocop:enable GraphQL/GraphqlName
       description 'Objects that can present an error'
 
-      field :code, Errors::ErrorCodeType, null: false, description: 'The code representing the error type'
       field :details, [Errors::DetailedErrorType], null: true, description: 'Detailed validation errors if applicable'
+      field :error_code, ErrorCodeEnum, null: false, description: 'The code representing the error type'
+
     end
   end
 end
diff --git a/app/services/error_code.rb b/app/services/error_code.rb
@@ -36,7 +36,8 @@ def self.error_codes
       failed_to_invalidate_old_backup_codes: { description: 'The old backup codes could not be deleted' },
       failed_to_save_valid_backup_code: { description: 'The new backup codes could not be saved' },
       invalid_setting: { description: 'Invalid setting provided' },
-
+      invalid_user: { description: 'The user is invalid because of active model errors' },
+      invalid_password_repeat: { description: 'The provided password repeat does not match the password' },
       primary_level_not_found: { description: '', deprecation_reason: 'Outdated concept' },
       secondary_level_not_found: { description: '', deprecation_reason: 'Outdated concept' },
       tertiary_level_exceeds_parameters: { description: '', deprecation_reason: 'Outdated concept' },
diff --git a/app/services/service_response.rb b/app/services/service_response.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class ServiceResponse
+  include Code0::ZeroTrack::Loggable
+
   def self.success(message: nil, payload: nil)
     new(status: :success, message: message, payload: payload)
   end
@@ -10,7 +12,8 @@ def self.error(message: nil, error_code: nil, details: nil)
 
     ErrorCode.validate_error_code!(error_code)
 
-    new(status: :error, message: message, payload: { error_code: error_code, details: details })
+    new(status: :error, message: message,
+        payload: { error_code: error_code, details: details })
   end
 
   attr_reader :status, :message, :payload
@@ -47,17 +50,16 @@ def to_h
 
   def to_mutation_response(success_key: :object)
     return { success_key => payload, errors: [] } if success?
-    return { success_key => nil, errors: payload } if payload&.details.is_a?(ActiveModel::Errors)
-
-    payload.details = Array.wrap(payload&.details).map do |message|
-      case message
-      when String
-        Sagittarius::Graphql::ErrorMessageContainer.new(message: message)
-      when Symbol
-        Sagittarius::Graphql::ServiceResponseErrorContainer.new(error_code: message)
-      end
-    end
 
-    { success_key => nil, errors: payload }
+    payload[:details] = if payload[:details].is_a?(ActiveModel::Errors)
+                          payload[:details].errors
+                        else
+                          Array.wrap(payload[:details]).map do |message|
+                            { message: message }
+                          end
+                        end
+
+    { success_key => nil,
+      errors: [Sagittarius::Graphql::ErrorContainer.new(payload[:error_code], payload[:details])] }
   end
 end
diff --git a/app/services/users/identity/register_service.rb b/app/services/users/identity/register_service.rb
@@ -16,14 +16,14 @@ def initialize(provider_id, args)
 
       def execute
         unless ApplicationSetting.current[:user_registration_enabled]
-          return ServiceResponse.error(message: 'User registration is disabled', payload: :registration_disabled)
+          return ServiceResponse.error(message: 'User registration is disabled', error_code: :registration_disabled)
         end
 
         begin
           identity = identity_provider.load_identity(provider_id, args)
         rescue Code0::Identities::Error => e
           logger.warn(message: 'Identity validation failed', exception: e)
-          return ServiceResponse.error(message: e.message, payload: :identity_validation_failed)
+          return ServiceResponse.error(message: e.message, error_code: :identity_validation_failed)
         end
 
         identifier = identity.identifier
@@ -33,7 +33,7 @@ def execute
         lastname = identity.lastname
         password = SecureRandom.base58(50)
 
-        return ServiceResponse.error(message: 'No email given', payload: :missing_identity_data) if email.nil?
+        return ServiceResponse.error(error_code: :missing_identity_data) if email.nil?
 
         username = email.split('@').first if username.nil?
 
@@ -48,17 +48,17 @@ def execute
           user = User.create(username: username, email: email, password: password, firstname: firstname,
                              lastname: lastname)
           user.ensure_namespace
-          return ServiceResponse.error(message: 'User is invalid', payload: user.errors) unless user.persisted?
+          return ServiceResponse.error(error_code: :invalid_user, details: user.errors) unless user.persisted?
 
           user_identity = UserIdentity.create(user: user, provider_id: provider_id, identifier: identifier)
           unless user_identity.persisted?
-            t.rollback_and_return! ServiceResponse.error(message: 'UserIdentity is invalid',
-                                                         payload: user_identity.errors)
+            t.rollback_and_return! ServiceResponse.error(error_code: :invalid_user_identity,
+                                                         details: user_identity.errors)
           end
           user_session = UserSession.create(user: user)
           unless user_session.persisted?
-            t.rollback_and_return! ServiceResponse.error(message: 'UserSession is invalid',
-                                                         payload: user_session.errors)
+            t.rollback_and_return! ServiceResponse.error(message: :invalid_user_session,
+                                                         details: user_session.errors)
           end
 
           AuditService.audit(
diff --git a/app/services/users/register_service.rb b/app/services/users/register_service.rb
@@ -15,18 +15,21 @@ def initialize(username, email, password)
 
     def execute
       unless ApplicationSetting.current[:user_registration_enabled]
-        return ServiceResponse.error(message: 'User registration is disabled', payload: :registration_disabled)
+        return ServiceResponse.error(message: 'User registration is disabled', error_code: :registration_disabled)
       end
 
       transactional do |t|
         user = User.create(username: username, email: email, password: password)
         user.ensure_namespace
-        return ServiceResponse.error(message: 'User is invalid', payload: user.errors) unless user.persisted?
+        unless user.persisted?
+          return ServiceResponse.error(message: 'User is invalid', error_code: :invalid_user, details: user.errors)
+        end
 
         user_session = UserSession.create(user: user)
         unless user_session.persisted?
           t.rollback_and_return! ServiceResponse.error(message: 'UserSession is invalid',
-                                                       payload: user_session.errors)
+                                                       error_code: :invalid_user_session,
+                                                       details: user_session.errors)
         end
 
         email_verification_response = EmailVerificationSendService.new(
@@ -36,7 +39,8 @@ def execute
 
         unless email_verification_response.success?
           t.rollback_and_return! ServiceResponse.error(message: 'Failed to send verification email',
-                                                       payload: email_verification_response.payload)
+                                                       error_code: :email_verification_send_failed,
+                                                       details: email_verification_response.payload)
         end
 
         AuditService.audit(
diff --git a/lib/sagittarius/graphql/error_container.rb b/lib/sagittarius/graphql/error_container.rb
@@ -2,6 +2,6 @@
 
 module Sagittarius
   module Graphql
-    ErrorMessageContainer = Struct.new(:message)
+    ErrorContainer = Struct.new(:error_code, :details)
   end
 end
diff --git a/lib/sagittarius/graphql/service_response_error_container.rb b/lib/sagittarius/graphql/service_response_error_container.rb
diff --git a/spec/graphql/types/errors/detailed_error_type_spec.rb b/spec/graphql/types/errors/detailed_error_type_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe SagittariusSchema.types['DetailedError'] do
+  it 'returns possible types' do
+    expect(described_class.possible_types).to include(
+      Types::Errors::ActiveModelErrorType,
+      Types::Errors::MessageErrorType
+    )
+  end
+
+  describe '.resolve_type' do
+    it 'resolves active model errors' do
+      expect(
+        described_class.resolve_type(ActiveModel::Error.new(nil, :test, :invalid), {})
+      ).to eq(Types::Errors::ActiveModelErrorType)
+    end
+
+    it 'resolves message errors' do
+      expect(
+        described_class.resolve_type({ message: 'message' }, {})
+      ).to eq(Types::Errors::MessageErrorType)
+    end
+
+    it 'raises an error for invalid types' do
+      expect { described_class.resolve_type(build(:user), {}) }.to raise_error 'Unsupported DetailedErrorType'
+    end
+  end
+end
diff --git a/spec/graphql/types/errors/error_type_spec.rb b/spec/graphql/types/errors/error_type_spec.rb
@@ -3,28 +3,13 @@
 require 'rails_helper'
 
 RSpec.describe SagittariusSchema.types['Error'] do
-  it 'returns possible types' do
-    expect(described_class.possible_types).to include(
-      Types::Errors::ActiveModelErrorType,
-      Types::Errors::MessageErrorType
-    )
+  let(:fields) do
+    %w[
+      errorCode
+      details
+    ]
   end
 
-  describe '.resolve_type' do
-    it 'resolves active model errors' do
-      expect(
-        described_class.resolve_type(ActiveModel::Error.new(nil, :test, :invalid), {})
-      ).to eq(Types::Errors::ActiveModelErrorType)
-    end
-
-    it 'resolves message errors' do
-      expect(
-        described_class.resolve_type(Sagittarius::Graphql::ErrorMessageContainer.new(message: 'message'), {})
-      ).to eq(Types::Errors::MessageErrorType)
-    end
-
-    it 'raises an error for invalid types' do
-      expect { described_class.resolve_type(build(:user), {}) }.to raise_error 'Unsupported ErrorType'
-    end
-  end
+  it { expect(described_class.graphql_name).to eq('Error') }
+  it { expect(described_class).to have_graphql_fields(fields) }
 end
