Merge pull request #8918 from tchak/graphql-add-error-codes

ETQ intégrateur API, je voudrais avoir des codes d'erreur plus précis

app/controllers/api/v2/graphql_controller.rb

@@ -5,7 +5,9 @@ class API::V2::GraphqlController < API::V2::BaseController
 
     render json: result
   rescue GraphQL::ParseError, JSON::ParserError => exception
-    handle_parse_error(exception)
+    handle_parse_error(exception, :graphql_parse_failed)
+  rescue ArgumentError => exception
+    handle_parse_error(exception, :bad_request)
   rescue => exception
     if Rails.env.production?
       handle_error_in_production(exception)
@@ -33,7 +35,12 @@ class API::V2::GraphqlController < API::V2::BaseController
   rescue ActionDispatch::Http::Parameters::ParseError => exception
     render json: {
       errors: [
-        { message: exception.cause.message }
+        {
+          message: exception.cause.message,
+          extensions: {
+            code: :bad_request
+          }
+        }
       ],
       data: nil
     }, status: 400
@@ -75,10 +82,13 @@ class API::V2::GraphqlController < API::V2::BaseController
     end
   end
 
-  def handle_parse_error(exception)
+  def handle_parse_error(exception, code)
     render json: {
       errors: [
-        { message: exception.message }
+        {
+          message: exception.message,
+          extensions: { code: }
+        }
       ],
       data: nil
     }, status: 400
@@ -90,22 +100,32 @@ class API::V2::GraphqlController < API::V2::BaseController
 
     render json: {
       errors: [
-        { message: exception.message, backtrace: exception.backtrace }
+        {
+          message: exception.message,
+          extensions: {
+            code: :internal_server_error,
+            backtrace: exception.backtrace
+          }
+        }
       ],
       data: nil
     }, status: 500
   end
 
   def handle_error_in_production(exception)
-    extra = { exception_id: SecureRandom.uuid }
+    exception_id = SecureRandom.uuid
-    Sentry.capture_exception(exception, extra:)
+    Sentry.with_scope do |scope|
+      scope.set_tags(exception_id:)
+      Sentry.capture_exception(exception)
+    end
 
     render json: {
       errors: [
         {
           message: "Internal Server Error",
           extensions: {
-            exception: { id: extra[:exception_id] }
+            code: :internal_server_error,
+            exception_id:
           }
         }
       ],

app/graphql/api/v2/schema.rb

@@ -25,8 +25,6 @@ class API::V2::Schema < GraphQL::Schema
 
   def self.object_from_id(id, ctx)
     ApplicationRecord.record_from_typed_id(id)
-  rescue => e
-    raise GraphQL::ExecutionError.new(e.message, extensions: { code: :not_found })
   end
 
   def self.resolve_type(type_definition, object, ctx)
@@ -50,7 +48,7 @@ class API::V2::Schema < GraphQL::Schema
     when GroupeInstructeur
       Types::GroupeInstructeurType
     else
-      raise GraphQL::ExecutionError.new("Unexpected object: #{object}")
+      type_definition
     end
   end
 
@@ -129,8 +127,13 @@ class API::V2::Schema < GraphQL::Schema
     super
   end
 
+  rescue_from(ActiveRecord::RecordNotFound) do |_error, _object, _args, _ctx, field|
+    raise GraphQL::ExecutionError.new("#{field.type.unwrap.graphql_name} not found", extensions: { code: :not_found })
+  end
+
   class Timeout < GraphQL::Schema::Timeout
     def handle_timeout(error, query)
+      error.extensions = { code: :timeout }
       Sentry.capture_exception(error, extra: query.context.query_info)
     end
   end

app/graphql/api/v2/stored_query.rb

@@ -8,7 +8,7 @@ class API::V2::StoredQuery
     when 'introspection'
       GraphQL::Introspection::INTROSPECTION_QUERY
     else
-      raise GraphQL::ExecutionError.new("No query with id \"#{query_id}\"")
+      raise GraphQL::ExecutionError.new("No query with id \"#{query_id}\"", extensions: { code: :bad_request })
     end
   end
 

app/graphql/loaders/champ.rb

@@ -13,7 +13,7 @@ module Loaders
 
     def perform(keys)
       query(keys).each { |record| fulfill(record.stable_id, [record].compact) }
-      keys.each { |key| fulfill(key, nil) unless fulfilled?(key) }
+      keys.each { |key| fulfill(key, []) unless fulfilled?(key) }
     end
 
     private

app/graphql/loaders/record.rb

@@ -21,7 +21,7 @@ module Loaders
         fulfilled_value = @array ? [record].compact : record
         fulfill(record.public_send(@column), fulfilled_value)
       end
-      keys.each { |key| fulfill(key, nil) unless fulfilled?(key) }
+      keys.each { |key| fulfill(key, @array ? [] : nil) unless fulfilled?(key) }
     end
 
     private

app/graphql/types/base_object.rb

@@ -1,5 +1,11 @@
 module Types
   class BaseObject < GraphQL::Schema::Object
     field_class BaseField
+
+    class InvalidNullError < GraphQL::InvalidNullError
+      def to_h
+        super.merge(extensions: { code: :invalid_null })
+      end
+    end
   end
 end

app/graphql/types/query_type.rb

@@ -26,13 +26,11 @@ module Types
       demarche_number = demarche.number.presence || ApplicationRecord.id_from_typed_id(demarche.id)
       Procedure
         .includes(draft_revision: :procedure, published_revision: :procedure)
-        .find_by(id: demarche_number)
+        .find(demarche_number)
     end
 
     def demarche(number:)
       Procedure.for_api_v2.find(number)
-    rescue => e
-      raise GraphQL::ExecutionError.new(e.message, extensions: { code: :not_found })
     end
 
     def dossier(number:)
@@ -42,14 +40,10 @@ module Types
         Dossier.visible_by_administration.for_api_v2.find(number)
       end
       DossierPreloader.load_one(dossier)
-    rescue => e
-      raise GraphQL::ExecutionError.new(e.message, extensions: { code: :not_found })
     end
 
     def groupe_instructeur(number:)
       GroupeInstructeur.for_api_v2.find(number)
-    rescue => e
-      raise GraphQL::ExecutionError.new(e.message, extensions: { code: :not_found })
     end
 
     def self.accessible?(context)

spec/controllers/api/v2/graphql_controller_spec.rb

@@ -684,7 +684,7 @@ describe API::V2::GraphqlController do
           end
 
           it "should return an error" do
-            expect(gql_errors).to eq([{ message: "Cannot return null for non-nullable field PersonneMorale.siegeSocial" }])
+            expect(gql_errors).to eq([{ message: "Cannot return null for non-nullable field PersonneMorale.siegeSocial", extensions: { code: "invalid_null" } }])
           end
         end
       end

spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb

@@ -64,6 +64,18 @@ describe API::V2::GraphqlController do
       }
     end
 
+    context 'timeout' do
+      let(:variables) { { dossierNumber: dossier.id } }
+      let(:operation_name) { 'getDossier' }
+
+      before { allow_any_instance_of(API::V2::Schema::Timeout).to receive(:max_seconds).and_return(0) }
+
+      it {
+        expect(gql_errors.first[:message]).to eq('Timeout on Query.dossier')
+        expect(gql_errors.first[:extensions]).to eq({ code: 'timeout' })
+      }
+    end
+
     context 'getDossier' do
       let(:variables) { { dossierNumber: dossier.id } }
       let(:operation_name) { 'getDossier' }
@@ -76,6 +88,15 @@ describe API::V2::GraphqlController do
         expect(gql_data[:dossier][:demandeur][:prenom]).to eq(dossier.individual.prenom)
       }
 
+      context 'not found' do
+        let(:variables) { { dossierNumber: 0 } }
+
+        it {
+          expect(gql_errors.first[:message]).to eq('Dossier not found')
+          expect(gql_errors.first[:extensions]).to eq({ code: 'not_found' })
+        }
+      end
+
       context 'with entreprise' do
         let(:procedure) { create(:procedure, :published, :with_service, administrateurs: [admin], types_de_champ_public:) }
         let(:dossier) { create(:dossier, :en_construction, :with_entreprise, procedure: procedure) }
@@ -114,6 +135,15 @@ describe API::V2::GraphqlController do
         expect(gql_data[:demarche][:dossiers]).to be_nil
       }
 
+      context 'not found' do
+        let(:variables) { { demarcheNumber: 0 } }
+
+        it {
+          expect(gql_errors.first[:message]).to eq('Demarche not found')
+          expect(gql_errors.first[:extensions]).to eq({ code: 'not_found' })
+        }
+      end
+
       context 'include Dossiers' do
         let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true } }
 
@@ -182,6 +212,15 @@ describe API::V2::GraphqlController do
         expect(gql_data[:groupeInstructeur][:dossiers]).to be_nil
       }
 
+      context 'not found' do
+        let(:variables) { { groupeInstructeurNumber: 0 } }
+
+        it {
+          expect(gql_errors.first[:message]).to eq('GroupeInstructeurWithDossiers not found')
+          expect(gql_errors.first[:extensions]).to eq({ code: 'not_found' })
+        }
+      end
+
       context 'include Dossiers' do
         let(:variables) { { groupeInstructeurNumber: groupe_instructeur.id, includeDossiers: true } }
 
@@ -241,6 +280,15 @@ describe API::V2::GraphqlController do
         }
       end
 
+      context 'not found' do
+        let(:variables) { { demarche: { number: 0 } } }
+
+        it {
+          expect(gql_errors.first[:message]).to eq('DemarcheDescriptor not found')
+          expect(gql_errors.first[:extensions]).to eq({ code: 'not_found' })
+        }
+      end
+
       context 'find by id' do
         let(:variables) { { demarche: { id: procedure.to_typed_id } } }
 

spec/graphql/dossier_spec.rb

@@ -75,6 +75,27 @@ RSpec.describe Types::DossierType, type: :graphql do
     end
   end
 
+  describe 'dossier with selected champ' do
+    let(:procedure) { create(:procedure, types_de_champ_public: [{ libelle: 'yolo' }, { libelle: 'toto' }]) }
+    let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure:) }
+    let(:query) { DOSSIER_WITH_SELECTED_CHAMP_QUERY }
+    let(:variables) { { number: dossier.id, id: champ.to_typed_id } }
+    let(:champ) { dossier.champs_public.last }
+
+    context 'when champ exists' do
+      it {
+        expect(data[:dossier][:champs].size).to eq 1
+        expect(data[:dossier][:champs][0][:label]).to eq "toto"
+      }
+    end
+
+    context "when champ dosen't exists" do
+      let(:variables) { { number: dossier.id, id: '1234' } }
+
+      it { expect(data[:dossier][:champs].size).to eq 0 }
+    end
+  end
+
   describe 'dossier with conditional champs' do
     include Logic
     let(:stable_id) { 1234 }
@@ -390,4 +411,15 @@ RSpec.describe Types::DossierType, type: :graphql do
     }
   }
   GRAPHQL
+
+  DOSSIER_WITH_SELECTED_CHAMP_QUERY = <<-GRAPHQL
+  query($number: Int!, $id: ID!) {
+    dossier(number: $number) {
+      champs(id: $id) {
+        id
+        label
+      }
+    }
+  }
+  GRAPHQL
 end