Package extraction workers catch more errors
What does this MR do?
NuGet and RubyGems packages both require processing before they can be used in the GitLab registry. We have an ExtractionWorker
for each package type that opens them up, parses their metadata, and checks their general validity. If something goes wrong during this process we update the package record in the database to have a :status
of :error
.
It is difficult to account for every possible error in the extraction process because there are a lot of services and libs/gems being used. We want to avoid a situation where a worker fails, but the package status is not updated to :error
(it starts out with a status of :processing
).
This MR updates the rescue for each extraction worker so that if any StandardError
occurs, we update the package to have error
status and log the exception so we have a better understanding of what might cause these services to fail.
Screenshots (strongly suggested)
I manually tested this by creating a fake NuGet package and uploading it to the registry:
$ touch foo.nupkg
$ echo "not a package" >> foo.nupkg
$ nuget source Add -Name localh -Source "http://gdk.test:3001/api/v4/projects/82/packages/nuget/index.json" -UserName root -Password $TOKEN
Package source with Name: localh added successfully.
$ nuget push foo.nupkg -Source localh
Pushing foo.nupkg to 'http://gdk.test:3001/api/v4/projects/82/packages/nuget'...
PUT http://gdk.test:3001/api/v4/projects/82/packages/nuget/
Created http://gdk.test:3001/api/v4/projects/82/packages/nuget/ 3514ms
Your package was pushed.
Then after the background job had failed, I checked the package status in the database (2
is processing, 3
is error):
Before the change:
gitlabhq_development# select * from packages_packages where project_id = 82
;
id │ project_id │ created_at │ updated_at │ name │ version │ package_type │ creator_id │ status
═════╪════════════╪═══════════════════════════════╪═══════════════════════════════╪═════════════════════════╪════════════════════════════════════════════╪══════════════╪════════════╪════════
164 │ 82 │ 2021-05-13 15:24:37.610836-06 │ 2021-05-13 15:24:37.610836-06 │ NuGet.Temporary.Package │ 0.0.0-ff77e86a-aa80-4540-b6c4-602011d9e849 │ 4 │ 1 │ 2
After the change:
gitlabhq_development# select * from packages_packages where project_id = 82
;
id │ project_id │ created_at │ updated_at │ name │ version │ package_type │ creator_id │ status
═════╪════════════╪═══════════════════════════════╪═══════════════════════════════╪═════════════════════════╪════════════════════════════════════════════╪══════════════╪════════════╪════════
165 │ 82 │ 2021-05-13 15:26:19.608985-06 │ 2021-05-13 15:26:19.608985-06 │ NuGet.Temporary.Package │ 0.0.0-8cc3b64d-1b0f-4acc-b3ba-2f2cd17e3e01 │ 4 │ 1 │ 3
For rubygems, things were a bit trickier because the rubygems client stops you from pushing an obviously non-gem file. So I was able to use curl to directly publish the package using the same API that the Rubygems client would:
$ touch mygem_0.0.1.gem
$ echo "not a package" >> mygem_0.0.1.gem
$ curl --request POST \
> --upload-file mygem_0.0.1.gem \
> --header "Authorization:$TOKEN" \
> "http://gdk.test:3001/api/v4/projects/82/packages/rubygems/api/v1/gems"
{"message":"201 Created"}
And then checked the database for the package status
Before the change:
gitlabhq_development# select * from packages_packages where project_id = 82
;
id │ project_id │ created_at │ updated_at │ name │ version │ package_type │ creator_id │ status
═════╪════════════╪═══════════════════════════════╪═══════════════════════════════╪═════════════════════════╪════════════════════════════════════════════╪══════════════╪════════════╪════════
167 │ 82 │ 2021-05-13 15:21:53.365895-06 │ 2021-05-13 15:21:53.365895-06 │ Gem.Temporary.Package │ 0.0.0-87c46176-0b57-45c9-9cb5-f48b94e3eb32 │ 10 │ 1 │ 2
After the change:
gitlabhq_development# select * from packages_packages where project_id = 82
;
id │ project_id │ created_at │ updated_at │ name │ version │ package_type │ creator_id │ status
═════╪════════════╪═══════════════════════════════╪═══════════════════════════════╪═════════════════════════╪════════════════════════════════════════════╪══════════════╪════════════╪════════
168 │ 82 │ 2021-05-13 15:29:38.122801-06 │ 2021-05-13 15:29:38.122801-06 │ Gem.Temporary.Package │ 0.0.0-7e0b8c1a-a573-43fa-8b8d-622df58f7553 │ 10 │ 1 │ 3
Does this MR meet the acceptance criteria?
Conformity
-
I have included a changelog entry, or it's not needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
Related to #324206 (closed)