tus-node-server icon indicating copy to clipboard operation
tus-node-server copied to clipboard

GCS and S3 bucket inexistence handling

Open mitjap opened this issue 1 year ago • 3 comments

I'm opening a discussion that started in #299 about bucket existence checking. GCSDataStore does this asynchronously in constructor which by definition can not be async and any proper handling would cause race condition. If there is any error with this check or bucket does not exists all thrown errors are unhandled and usually causes application to exit with error code (crash).
https://github.com/tus/tus-node-server/blob/9eeb26c40042f1f95ccba71f0ff51f792ca0c6ac/lib/stores/GCSDataStore.js#L47

S3Store handles bucket existence when uploading a file and in case of an error signals this in response with status code 500. https://github.com/tus/tus-node-server/blob/9eeb26c40042f1f95ccba71f0ff51f792ca0c6ac/lib/stores/S3Store.js#L88

mitjap avatar Sep 30 '22 20:09 mitjap

I think it makes most sense to align with the S3 store here. During the initial POST it seems reasonable to find out at that time the bucket doesn't exist.

Murderlon avatar Oct 03 '22 13:10 Murderlon

POST is optional extension and might not be used.

I recommend testing at startup and save result as a promise which is always evaluated on every request. If initial test failed, all subsequest requests will fail. Another option is to just ignore this check (we can still print a warning to logs) and let it error on later requests. For example GCS reports that IAM does not have sufficient access if bucket does not exists when creating new object (on upload creation).

mitjap avatar Oct 04 '22 20:10 mitjap

I mean the initial POST to S3 not to the tus server, which is how it currently works for the S3 store, and apply that to GCS too. The way I see it, is that setting up a bucket is a prerequisite and we don't have to go too fancy.

Murderlon avatar Oct 04 '22 20:10 Murderlon