Somehow this implies the Ci::Build#options is returning a String instead of a Hash? I'm not sure what changed here. No new deploys went out, as far as I can tell.
@stanhu First time the error was seen on Sentry 3 days ago, around the latest GitLab.com deployment. Since then it was seen ~3k times at first day, only ~200 times yesterday and today there is a huge spike.
The problem seems to be caused by either a configuration or application change introduced with the deplpoy.
With the risk of going totally off-track, I think this might be related to #34711 (closed), where things for imported projects have strings of the hashes instead of directly the hashes
(tested this on a local instance after importing a project with a build from Auto DevOps):
It's sort of confusing why the data in these records is not being deserialized. Can someone with access to prod help us resolve that? In all of our experiments, we're unable to get a string in there -- even using the example that @cirimie provided. We have a fix in place, but if there's a way for us to parse this string that's probably a better fix.
@sabrams and I shipped a proposed fix and test for this bad data scenario we're seeing with !19122 (closed). @stanhu, it sounds like there's a process to resolve the issues with the data coming in? I'd say we can merge/ship this quick fix and open an issue to revert it once we have better data integrity.
CommitStatus is the base class, and Ci::Build inherits from it. However, only the latter serializes the options column, while CommitStatus stores it as raw text.
As a result, calling assign_attributes with an options hash on a CommitStatus object doesn't work because the column will write the data out as a string. To reproduce this problem, I did the following:
Copied the suspect project.json into spec/fixtures/lib/gitlab/import_export/project.json.
Applied the patch (see below) to enable debugging. Here you see the problem:
[298,307]in/Users/stanhu/gitlab/gdk-ee/gitlab/lib/gitlab/import_export/relation_factory.rb298:object=relation_class.new299:300:byebugif@debug301:# Use #assign_attributes here to call object custom setters302:object.assign_attributes(parsed_relation_hash)=>303:object304:end305:end306:end307:(byebug)object.options"{\"image\"=>{\"name\"=>\"busybox:latest\"}, \"before_script\"=>[\"echo \\\"Before script section\\\"\", \"echo \\\"For example you might run an update here or install a build dependency\\\"\", \"echo \\\"Or perhaps you might print out some debugging details\\\"\"], \"script\"=>[\"echo \\\"Do your build here\\\"\"], \"after_script\"=>[\"echo \\\"After script section\\\"\", \"echo \\\"For example you might do some cleanup here\\\"\"]}"(byebug)object.options.classString(byebug)parsed_relation_hash['options']{"image"=>{"name"=>"busybox:latest"},"before_script"=>["echo \"Before script section\"","echo \"For example you might run an update here or install a build dependency\"","echo \"Or perhaps you might print out some debugging details\""],"script"=>["echo \"Do your build here\""],"after_script"=>["echo \"After script section\"","echo \"For example you might do some cleanup here\""]}(byebug)parsed_relation_hash['options'].classHash
Why is this happening now?
I'm not sure. I suspect our project export JSON may have changed in a way that CommitStatus is assigned the options data. Either the statuses or the options was included in newly-exported project.json files, or we just got lucky for a while that no one imported pending pipelines (doubtful).
Appendix
This is the patch to reproduce the problem:
diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rbindex 3fa5765fd4a..d3650c63dbc 100644--- a/lib/gitlab/import_export/project_tree_restorer.rb+++ b/lib/gitlab/import_export/project_tree_restorer.rb@@ -198,6 +198,10 @@ module Gitlab def process_sub_relation(relation_key, relation_definition, relation_item) relation_definition.each do |sub_relation_key, sub_relation_definition|+ puts "relation is #{sub_relation_key}"+ if sub_relation_key.to_s == 'statuses' && relation_item['statuses'].first['status'] == 'pending'+ @debug = true+ end # We just use author to get the user ID, do not attempt to create an instance. next if sub_relation_key == :author@@ -225,7 +229,8 @@ module Gitlab merge_requests_mapping: merge_requests_mapping, user: @user, project: @project,- excluded_keys: excluded_keys_for_relation(relation_key))+ excluded_keys: excluded_keys_for_relation(relation_key),+ debug: @debug) end.compact relation_hash_list.is_a?(Array) ? relation_array : relation_array.firstdiff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rbindex cb85af91f75..45639c1d881 100644--- a/lib/gitlab/import_export/relation_factory.rb+++ b/lib/gitlab/import_export/relation_factory.rb@@ -55,7 +55,7 @@ module Gitlab relation_name.to_s.constantize end- def initialize(relation_sym:, relation_hash:, members_mapper:, merge_requests_mapping:, user:, project:, excluded_keys: [])+ def initialize(relation_sym:, relation_hash:, members_mapper:, merge_requests_mapping:, user:, project:, excluded_keys: [], debug: false) @relation_name = self.class.overrides[relation_sym]&.to_sym || relation_sym @relation_hash = relation_hash.except('noteable_id') @members_mapper = members_mapper@@ -63,6 +63,7 @@ module Gitlab @user = user @project = project @imported_object_retries = 0+ @debug = debug @relation_hash['project_id'] = @project.id@@ -105,6 +106,8 @@ module Gitlab when :notes then setup_note end+ byebug if @debug+ update_user_references update_project_references update_group_references@@ -294,6 +297,7 @@ module Gitlab else object = relation_class.new+ byebug if @debug # Use #assign_attributes here to call object custom setters object.assign_attributes(parsed_relation_hash) object
This changes the ClassType.new(attributes) to ClassType.new + assign_attributes,
The type that is disected from statuses: is CommitStatus,
It means that there's a problem with handling of type column, as Type.new(type: can properly mutate type into CommitStatus/GenericCommitStatus/Ci::Build, and pass attributes to create a correct type using the serialised attributes,
Now, when we have the CommitStatus, we do save it in database. However, the representation is not saved as a way that we want that, with using serialize, because serialize is defined on Ci::Build, not CommitStatus. It means that we do not restore/save all other relations like tags, that are on Ci::Build,
Now, when we read it from database we use a proper type, but structure of data is wrong as it did not go via proper handler.
The way to solve that would be likely this (still hacky):
## Before:CommitStatus.new(type: 'Ci::Build').class=>Ci::Build## After the change:CommitStatus.new.tap{|c|c.assign_attributes(type: 'Ci::Build')}.class=>CommitStatus
It means that currently import is severely broken, as it can restore some inherited types, as long as they do not have some special handling, like serialize implemented on subclass.
Had to make a change to !19122 (closed), but it seems like it's a valid short term fix if we want that. !19124 (merged) will improve the data integrity. And it seems like we need a data migration to clean up the data overall and a rollback of the changes we're introducing in !19122 (closed).
I don't know about the !19122 (closed), as this problem seems to be also for DiffNote. Can we focus on @stanhu fix and figure out a different way to fix the data? Maybe having rake script, or another sort of script to run it once and forget (if this is needed)?
The fix I'm trying to get merged will actually have an immediate impact on gitlab-com/gl-infra/production#1275 (comment 235337082), and what users are seeing. !19124 (merged) is the solution that needs to be made, but I don't see how it'll make any immediate impact to the issue that users are seeing.
Maybe worth noting that I'm only contributing to this because it was escalated to me as the on call engineer. Roll with whatever seems best, I'll leave y'all to it.
Stan Huchanged title from Error 500 in /api/v4/jobs/requests: NoMethodError: undefined method dig' for #<String:0x00007f72dff1b860>** to **Error 500 in /api/v4/jobs/request: NoMethodError: undefined method dig' for #String:0x00007f72dff1b860
changed title from Error 500 in /api/v4/jobs/requests: NoMethodError: undefined method dig' for #<String:0x00007f72dff1b860>** to **Error 500 in /api/v4/jobs/request: NoMethodError: undefined method dig' for #String:0x00007f72dff1b860