Joystream / joystream

Joystream Monorepo
http://www.joystream.org
GNU General Public License v3.0
1.42k stars 115 forks source link

[Colossus] storage failing to query data objects #5002

Closed yasiryagi closed 10 months ago

yasiryagi commented 10 months ago

Issue: when query data objects intermittently get error response detail:


~ # curl https://storage.freakstatic.com/storage/api/v1/state/data-objects
{"type":"request_exception","message":".response[565943] should be string"}
joystream ~ # curl https://joystream.joystream-arsi44.space/storage/api/v1/state/data-objects
{"type":"request_exception","message":".response[404642] should be string"}

Product: colossus Version: 3.9.0, 3.9.1

freakstatic commented 10 months ago

This seems to be happening on >= v3.9.0 I tested on 3.8.1 and it's not happening

freakstatic commented 10 months ago

v3.9.1 URL: https://joystream.joystream-arsi44.space/storage/api/v1/state/data-objects

mnaamani commented 10 months ago

This error is coming from response validation failing, we use Ajv package - the schema for the response is defined in:

https://github.com/Joystream/joystream/blob/10baf39c222ce42a7fc7ab70518835b00f67bdd8/storage-node/src/api-spec/openapi.yaml#L361

Somehow, the map is having a key set that is a number, not an string. Not sure how that is possible at least at compile time since the map is strongly typed. https://github.com/Joystream/joystream/blob/10baf39c222ce42a7fc7ab70518835b00f67bdd8/storage-node/src/services/caching/localDataObjects.ts#L11

I inspected all code paths that update the map and couldn't identify where we might be mistakenly setting a number.

Only thing I can think of is if somehow the spread operator used to generate the array: const ids = [...idCache.keys()] could be doing something funny on the node that has this issue?

Not sure if we wan't to investigate this further if we plan on removing this api endpoint see: https://github.com/Joystream/joystream/issues/5008 ?

mnaamani commented 10 months ago

Finally managed to find the fault. When HTTP GET or HEAD request is made to the node which does not have the object id, we hit these lines of code:

https://github.com/Joystream/joystream/blob/c4789f9bf556eee32e03853554c124631d34845c/storage-node/src/services/webApi/controllers/filesApi.ts#L65

https://github.com/Joystream/joystream/blob/c4789f9bf556eee32e03853554c124631d34845c/storage-node/src/services/webApi/controllers/filesApi.ts#L93

For the GET request, since the catch clause was executed because the file was not found, the request has already been handled and passed to the next handler, and the req.params.id is now undefined -> this pollutes the idCache with an undefined key, which results in response validation when fetching the data-objects endpoint. eg. .response[565943] should be string the number 565943 is the index into the array not the actual object id.

For the HEAD request, the behavior is different, during the unpinning the data object is now persisted in the idCache with a negative pin count.

It is easy to re-produce this by simply starting a test setup and doing GET and HEAD request to the node that does not have a particular object id.

Better checks as suggested in https://github.com/Joystream/joystream/issues/4991 should make any future issues easier to detect.

How I found the issue: Adding assert(typeof dataObjectId === 'string') statements in the functions that add and/or update entries.

2023-12-27 21:49:41:4941 debug: Query - storageBucketsConnection
2023-12-27 21:49:41:4941 http: HTTP POST /api/v1/files?dataObjectId=204&storageBucketId=1&bagId=static:council
2023-12-27 21:49:41:4941 http: HTTP GET /api/v1/files/204
2023-12-27 21:49:41:4941 error: GET /api/v1/files/204: Error 404: ENOENT: no such file or directory, open '/data/uploads/204'
/joystream/storage-node/src/services/caching/localDataObjects.ts:93
  assert(typeof dataObjectId === 'string')
        ^
AssertionError [ERR_ASSERTION]: false == true
    at unpinDataObjectIdFromCache (/joystream/storage-node/src/services/caching/localDataObjects.ts:93:9)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async getFile (/joystream/storage-node/src/services/webApi/controllers/filesApi.ts:65:5) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}
kdembler commented 10 months ago

closed by https://github.com/Joystream/joystream/pull/5016