@tus/s3-store: fix uncaught exception when cancelling an upload

[naumf]

Fixes #411

[Murderlon]

Thanks for the PR! Do you think we can test this easily in https://github.com/tus/tus-node-server/blob/main/packages/s3-store/test.ts? Would be nice a great case for a test.

[naumf]

I can't think of an easy way to test this.
I tested this with manual testing, once with the error event being handled and the other time the error event not being handled.

[Murderlon]

Perhaps we can do something similar to this:

tus-node-server/test/stores.test.ts

Lines 145 to 161 in ffb9f0c

	it('should reject when stream is destroyed', async function () {
	const file = new Upload({
	id: 'write-test-reject',
	size: this.testFileSize,
	offset: 0,
	metadata: {filename: 'world_domination_plan.pdf', is_confidential: null},
	})
	await this.datastore.create(file)
	const readable = new stream.Readable({
	read() {
	this.push('some data')
	this.destroy()
	},
	})
	const offset = this.datastore.write(readable, file.id, 0)
	return offset.should.be.rejected()
	})

Instead of destroy, we error and assert the error? Can we distinguish there between unexpected and expected?

[naumf]

I'm not sure if it is testable at all because of a possible race between reject and resolve:

• resolve will always be called
  https://github.com/tus/tus-node-server/blob/main/packages/s3-store/index.ts#L273
• if the error event fires, then reject will be called before or after resolve is called:
  https://github.com/tus/tus-node-server/pull/412/files#diff-4b59e755325ac2230c23dc3627830c0c1dd6331b299dffecc19ff24ede9a2277R266
  While not handling the event at all crashes the server, this will not crash it, but it makes the code not easy to test.

[Murderlon]

Thank you!

[Murderlon]

https://github.com/tus/tus-node-server/releases/tag/%40tus%2Fs3-store%401.0.0-beta.5