getodk / central-backend

Node.js based backend for ODK Central
https://docs.getodk.org/central-intro/
Apache License 2.0
50 stars 73 forks source link

Error: end of central directory record signature not found #595

Open matthew-white opened 2 years ago

matthew-white commented 2 years ago

There have been intermittent test failures with the error Error: end of central directory record signature not found. @ktuite and I have seen the error locally, and we also sometimes see the error on CircleCI. The error has been mentioned on the forum: https://forum.getodk.org/t/error-end-of-central-directory-record-signature-not-found/36182

We've seen two versions of the error:

Version 1:

     Error: end of central directory record signature not found
      at node_modules/yauzl/index.js:183:14
      at Immediate.<anonymous> (node_modules/yauzl/index.js:615:38)
      at processImmediate (internal/timers.js:464:21)

Version 2:

     Error: end of central directory record signature not found
      at node_modules/yauzl/index.js:183:14
      at node_modules/yauzl/index.js:622:5
      at node_modules/fd-slicer/index.js:32:7
      at FSReqCallback.wrapper [as oncomplete] (fs.js:561:5)

Version 1 of the error can be seen in this CircleCI build. I ran make test in CircleCI 200 times, and the error came up 6 times (all version 1). In CircleCI, we have seen the error in the following tests:

List ``` 1) zipPart streamer should close the archive only after parts are finalized: 1) managed encryption end-to-end should decrypt attached files successfully: 2) managed encryption end-to-end should handle mixed [plaintext/encrypted] attachments (not decrypting): 1) .zip attachments streaming should stream the contents to files at the appropriate paths: 1) api: /forms/:id/submissions [draft] .csv.zip should not carry over drafts when a draft is replaced: ```

@ktuite mentioned:

I occasionally see this Error: end of central directory record signature not found error on submission zip tests. Not all the time, not always the same test. But the tests all involve yauzl.

@ktuite has seen version 1 of the error in the following test:

  1) api: /forms/:id/submissions
       [draft] .csv.zip
         should return draft submissions:

I've seen version 2 of the error locally in the following tests:

List ``` 2) managed encryption end-to-end should decrypt over cookie auth with passphrases provided via url-encoded POST body: 1) managed encryption end-to-end should strip .enc suffix from decrypted attachments: 1) managed encryption end-to-end should decrypt attached files successfully: 2) managed encryption end-to-end should decrypt to CSV successfully: 1) zipPart streamer should close the archive only after parts are finalized: 2) .csv.zip briefcase output @slow should output all rows: 1) .csv.zip briefcase output @slow should attach attachments information if present: 1) api: /forms/:id/submissions .csv.zip GET versioning should return original instanceId and latest attached audit log when instanceId deprecated with new audit log: 1) api: /forms/:id/submissions .csv.zip GET should return consolidated client audit log filtered by user: 1) api: /forms/:id/submissions .csv.zip GET should return consolidated client audit log filtered by review state: 1) api: /forms/:id/submissions .csv.zip GET should not include data from other forms: 1) api: /forms/:id/submissions [draft] .csv.zip should not carry draft submissions forward to the published version upon publish: ```

The error mentioned on the forum is also version 2 of the error.

matthew-white commented 2 years ago

I think there's some issue with unzipping (either with the package we use or with how we use it). It's rare that we have to unzip in production. (Maybe backup restore is the only case?) But it's common for us to unzip in testing.

matthew-white commented 2 years ago

I ran make test in CircleCI another 100 times after merging #591. That PR upgraded Node and also bumped yauzl, a dependency seen in the stack traces above. However, I'm still seeing this error, so it doesn't seem to be specific to Node 14.

alxndrsn commented 1 year ago

Seen in CircleCI at https://app.circleci.com/pipelines/github/getodk/central-backend/1131/workflows/f9ff55b2-5136-4834-814d-edb7a1f7ab10/jobs/1912:

  1) managed encryption
       end-to-end
         should handle mixed[managedA/managedB] formdata (decrypting):
     Error: end of central directory record signature not found
      at /home/circleci/repo/node_modules/yauzl/index.js:187:14
      at Immediate.<anonymous> (node_modules/yauzl/index.js:624:38)
      at processImmediate (node:internal/timers:466:21)

make: *** [Makefile:35: test-full] Error 1
alxndrsn commented 1 year ago

New variation seen at https://app.circleci.com/pipelines/github/alxndrsn/odk-central-backend/182/workflows/25663dd0-3856-4c4a-9ebc-b132a26a39dc/jobs/189

  1 failing

  1) managed encryption
       end-to-end
         should handle mixed[plaintext/encrypted] formdata (not decrypting):
     Error: end of central directory record signature not found
      at /home/circleci/repo/node_modules/yauzl/index.js:187:14
      at Immediate.<anonymous> (node_modules/yauzl/index.js:624:38)
      at processImmediate (node:internal/timers:466:21)

make: *** [Makefile:39: test-full] Error 1

Exited with code exit status 2
alxndrsn commented 5 months ago

"end of central directory record signature not found" suggests that pZipStreamToFiles() (or maybe zipStreamToFiles()) is being provided a corrupt/invalid zipfile.

This suggests there was an error in the backend when constructing the zipfile.

This should be recreatable by throwing in e.g. Writable.write() in streamAttachments() in lib/data/attachments.js. There does not currently appear to be any logging of these errors.

Streaming responses are confusing in part because the response headers & status are written before the content of the response has been fetched from the DB / generated.