Merge pull request #8865 from tchak/graphql-improuve-logs

feat(graphql): improuve logs

app/controllers/api/v2/graphql_controller.rb

@@ -1,9 +1,7 @@
 class API::V2::GraphqlController < API::V2::BaseController
   def execute
-    result = API::V2::Schema.execute(query,
-      variables: variables,
-      context: context,
-      operation_name: params[:operationName])
+    result = API::V2::Schema.execute(query:, variables:, context:, operation_name:)
+    @query_info = result.context.query_info
 
     render json: result
   rescue GraphQL::ParseError, JSON::ParserError => exception
@@ -27,10 +25,7 @@ class API::V2::GraphqlController < API::V2::BaseController
 
     super
 
-    payload.merge!({
-      graphql_query: query(fallback: params[:queryId]),
-      graphql_variables: to_unsafe_hash(params[:variables]).to_json
-    })
+    payload.merge!(@query_info.presence || {})
   end
 
   def process_action(*args)
@@ -44,9 +39,9 @@ class API::V2::GraphqlController < API::V2::BaseController
     }, status: 400
   end
 
-  def query(fallback: nil)
+  def query
     if params[:queryId].present?
-      API::V2::StoredQuery.get(params[:queryId], fallback: fallback)
+      API::V2::StoredQuery.get(params[:queryId])
     else
       params[:query]
     end
@@ -56,6 +51,10 @@ class API::V2::GraphqlController < API::V2::BaseController
     ensure_hash(params[:variables])
   end
 
+  def operation_name
+    params[:operationName]
+  end
+
   # Handle form data, JSON body, or a blank value
   def ensure_hash(ambiguous_param)
     case ambiguous_param
@@ -76,23 +75,6 @@ class API::V2::GraphqlController < API::V2::BaseController
     end
   end
 
-  def to_unsafe_hash(ambiguous_param)
-    case ambiguous_param
-    when String
-      if ambiguous_param.present?
-        JSON.parse(ambiguous_param)
-      else
-        {}
-      end
-    when ActionController::Parameters
-      ambiguous_param.to_unsafe_h
-    else
-      ambiguous_param
-    end
-  rescue JSON::ParserError
-    {}
-  end
-
   def handle_parse_error(exception)
     render json: {
       errors: [
@@ -115,15 +97,15 @@ class API::V2::GraphqlController < API::V2::BaseController
   end
 
   def handle_error_in_production(exception)
-    id = SecureRandom.uuid
-    Sentry.capture_exception(exception, extra: { exception_id: id })
+    extra = { exception_id: SecureRandom.uuid }
+    Sentry.capture_exception(exception, extra:)
 
     render json: {
       errors: [
         {
           message: "Internal Server Error",
           extensions: {
-            exception: { id: id }
+            exception: { id: extra[:exception_id] }
           }
         }
       ],

app/graphql/api/v2/context.rb

@@ -55,6 +55,15 @@ class API::V2::Context < GraphQL::Query::Context
     self[:authorized][demarche.id]
   end
 
+  def query_info
+    {
+      graphql_query: query.query_string,
+      graphql_variables: query.provided_variables&.to_json,
+      graphql_null_error: errors.any? { _1.is_a? GraphQL::InvalidNullError }.presence,
+      graphql_timeout_error: errors.any? { _1.is_a? GraphQL::Schema::Timeout::TimeoutError }.presence
+    }.compact
+  end
+
   private
 
   def compute_demarche_authorization(demarche)

app/graphql/api/v2/schema.rb

@@ -125,14 +125,13 @@ class API::V2::Schema < GraphQL::Schema
   def self.type_error(error, ctx)
     # Capture type errors in Sentry. Thouse errors are our responsability and usually linked to
     # instances of "bad data".
-    Sentry.capture_exception(error)
+    Sentry.capture_exception(error, extra: ctx.query_info)
     super
   end
 
   class Timeout < GraphQL::Schema::Timeout
     def handle_timeout(error, query)
-      extra = { query: query.query_string, variables: query.provided_variables&.to_json }
-      Sentry.capture_exception(error, extra:)
+      Sentry.capture_exception(error, extra: query.context.query_info)
     end
   end
 

app/graphql/api/v2/stored_query.rb

@@ -1,5 +1,5 @@
 class API::V2::StoredQuery
-  def self.get(query_id, fallback: nil)
+  def self.get(query_id)
     case query_id
     when 'ds-query-v2'
       QUERY_V2
@@ -8,11 +8,7 @@ class API::V2::StoredQuery
     when 'introspection'
       GraphQL::Introspection::INTROSPECTION_QUERY
     else
-      if fallback.nil?
-        raise GraphQL::ExecutionError.new("No query with id \"#{query_id}\"")
-      else
-        fallback
-      end
+      raise GraphQL::ExecutionError.new("No query with id \"#{query_id}\"")
     end
   end
 

config/initializers/lograge.rb

@@ -16,6 +16,8 @@ Rails.application.configure do
         user_agent: event.payload[:user_agent],
         graphql_query: event.payload[:graphql_query],
         graphql_variables: event.payload[:graphql_variables],
+        graphql_null_error: event.payload[:graphql_null_error],
+        graphql_timeout_error: event.payload[:graphql_timeout_error],
         browser: event.payload[:browser],
         browser_version: event.payload[:browser_version],
         platform: event.payload[:platform],

spec/controllers/api/v2/graphql_controller_spec.rb

@@ -684,6 +684,16 @@ describe API::V2::GraphqlController do
             })
           end
         end
+
+        context "error in degraded mode" do
+          before do
+            dossier.etablissement.update!(adresse: nil, siege_social: nil)
+          end
+
+          it "should return an error" do
+            expect(gql_errors).to eq([{ message: "Cannot return null for non-nullable field PersonneMorale.siegeSocial" }])
+          end
+        end
       end
 
       context "champs" do