Handle a missing tarball attachment in npm package creation
What does this MR do and why?
Packages::Npm::CreatePackageService reads the tarball from params['_attachments'][package_file_name] (L108-116) and answers Attachment data is empty. when its data is blank (L20). A missing attachment raises NoMethodError instead:
- With the tarball under a name other than
<name>-<version>.tgz(for example without the scope),attachmentisniland the check raises. - Without
_attachments, readingattachmentraises before the check.
ProcessTemporaryPackageFileWorker rescues it, logs it and marks the package as failed with Unexpected error: NoMethodError.
This MR adds a check for a missing attachment, so the package fails with Attachment is missing. and no exception is logged; an attachment with empty data still fails with Attachment data is empty. as before. Only publishes that already fail change, and only in their status message. No API, GraphQL or database change.
References
- #403162 (closed) and !119209 (merged) added the check for an attachment with empty data.
- !215599 (merged) sends every npm publish through the worker.
I found this while working on gitlab-mcp-server.
Screenshots or screen recordings
Not applicable: no UI change.
How to set up and validate locally
-
Create
payload.json, with the tarball under the unscoped file name:{ "name": "@<group>/demo", "dist-tags": { "latest": "1.0.0" }, "versions": { "1.0.0": { "name": "@<group>/demo", "version": "1.0.0", "dist": { "shasum": "f572d396fae9206628714fb2ce00f72e94f2258f" } } }, "_attachments": { "demo-1.0.0.tgz": { "content_type": "application/octet-stream", "data": "aGVsbG8K", "length": 6 } } } -
Publish it to a project of the group
<group>:curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" --header "Content-Type: application/json" \ --data @payload.json --url "http://gdk.test:3000/api/v4/projects/<project_id>/packages/npm/@<group>%2Fdemo" -
Once Sidekiq has run the job, open Deploy > Package registry. On master the package shows
Unexpected error: NoMethodErrorandlog/exceptions_json.loghas the exception. On this branch it showsAttachment is missing.and no exception is logged.
Specs I ran (Ruby 3.3.11)
spec/services/packages/npm/create_package_service_spec.rbwith master's service: 85 examples, 4 failures, the four examples of the two new contexts, each withNoMethodError. On this branch, with the specs of the callers (ProcessTemporaryPackageFileService,CreateTemporaryPackageService,ProcessTemporaryPackageFileWorker,Packages::ErrorHandling): 152 examples, 0 failures.- A request spec kept out of this MR that publishes a payload like the one above through the API and runs the worker inline: on master
Unexpected error: NoMethodErrorwith the exception logged; on this branchAttachment is missing.and no exception logged. - The npm project request specs, CE and EE, at the first commit: 426 examples, 1 failure, a Snowplow expectation of the dist-tags endpoint that passes alone and does not reach this code.
- RuboCop on the two files: no offenses. The commit linter: no findings.
- Left to the pipeline: the npm group and instance request specs, and Ruby 3.4.10.
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.