Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onUploadFinish error throw not being returned to client #380

Open
bodhi-maligator opened this issue Jan 18, 2023 · 7 comments
Open

onUploadFinish error throw not being returned to client #380

bodhi-maligator opened this issue Jan 18, 2023 · 7 comments

Comments

@bodhi-maligator
Copy link

Hi,

Not sure if I'm missing something. I tried a simple server:

`'use strict'

import {Server} from '@tus/server';
import {FileStore} from '@tus/file-store';

const host = '127.0.0.1'
const port = 1080
const server = new Server({
path: '/uploads',
datastore: new FileStore({directory: './uploads'}),
async onUploadFinish(req, res, upload) {
throw {body: 'no', status_code: 500};
return res;
}
})

server.listen({host, port})`

And when I try with a simple tus client:

`'use strict'

import * as fs from 'node:fs';
import * as tus from 'tus-js-client';

const path = './example.doc';
const file = fs.createReadStream(path);

const options = {
endpoint: 'http://127.0.0.1:1080/uploads/',
metadata: {
filename: 'example.doc',
filetype: 'application/msword',
description: 'scooby doobie doo'
},
onError (error) {
console.error('An error occurred:')
console.error(error)
process.exitCode = 1
},
onProgress (bytesUploaded, bytesTotal) {
const percentage = ((bytesUploaded / bytesTotal) * 100).toFixed(2)
console.log(bytesUploaded, bytesTotal, ${percentage}%)
},
onSuccess () {
console.log('Upload finished:', upload.url)
},
};

const upload = new tus.Upload(file, options);
upload.start();`

The upload completes without error.

node ./tus-client.js
0 3690496 0.00%
65536 3690496 1.78%
3690496 3690496 100.00%
Upload finished: http://127.0.0.1:1080/uploads/b21c2577eee60411d30b9edc01d7b4ff

Any help would be appreciated!

thanks

@Murderlon
Copy link
Member

Hi, that is very surprising considering I have this end-to-end test:

it('should call onUploadFinish and return its error to the client with creation-with-upload ', (done) => {
const server = new Server({
path: '/test/output',
datastore: new FileStore({directory: './test/output'}),
async onUploadFinish() {
throw {body: 'no', status_code: 500}
},
})
request(server.listen())
.post(server.options.path)
.set('Tus-Resumable', TUS_RESUMABLE)
.set('Upload-Length', '4')
.set('Upload-Offset', '0')
.set('Content-Type', 'application/offset+octet-stream')
.send('test')
.expect(500, 'no', done)
})

I'll try to reproduce

@bodhi-maligator
Copy link
Author

I forget env details:

node: v16.15.1

lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description: Debian GNU/Linux 11 (bullseye)
Release: 11
Codename: bullseye

@bodhi-maligator
Copy link
Author

Running 'yarn test' resulted in:

@tus/server:test: ✔ should call onUploadFinish and return its error to the client
@tus/server:test: ✔ should call onUploadFinish and return its error to the client with creation-with-upload

@bodhi-maligator
Copy link
Author

I also ran the following:

`import * as fs from 'node:fs';
import fetch from 'node-fetch';

const path = 'const path = './example.doc';';
const file = fs.createReadStream(path);

const stats = fs.statSync(path);
const fileSizeInBytes = stats.size;

fetch('http://127.0.0.1:1080/uploads/', {
method: 'POST',
headers: {
'Content-length': fileSizeInBytes,
'Tus-Resumable': '1.0.0',
'Upload-Length': fileSizeInBytes,
'Content-Type': 'application/msword'
},
body: file
})
.then(function(res) {
console.log(res);
})
.catch((err) => {
console.log(err);
})
;`

which resulted in:

Response { size: 0, [Symbol(Body internals)]: { body: PassThrough { _readableState: [ReadableState], _events: [Object: null prototype], _eventsCount: 5, _maxListeners: undefined, _writableState: [WritableState], allowHalfOpen: true, [Symbol(kCapture)]: false, [Symbol(kCallback)]: null }, stream: PassThrough { _readableState: [ReadableState], _events: [Object: null prototype], _eventsCount: 5, _maxListeners: undefined, _writableState: [WritableState], allowHalfOpen: true, [Symbol(kCapture)]: false, [Symbol(kCallback)]: null }, boundary: null, disturbed: false, error: null }, [Symbol(Response internals)]: { type: 'default', url: 'http://127.0.0.1:1080/uploads/', status: 201, statusText: 'Created', headers: { 'access-control-expose-headers': 'Authorization, Content-Type, Location, Tus-Extension, Tus-Max-Size, Tus-Resumable, Tus-Version, Upload-Concat, Upload-Defer-Length, Upload-Length, Upload-Metadata, Upload-Offset, X-HTTP-Method-Override, X-Requested-With, X-Forwarded-Host, X-Forwarded-Proto, Forwarded', connection: 'close', 'content-length': '0', date: 'Wed, 25 Jan 2023 16:24:02 GMT', location: 'http://127.0.0.1:1080/uploads/df932034006a7138a37598e179d1f766', 'tus-resumable': '1.0.0' }, counter: 0, highWaterMark: 16384 } }

@bodhi-maligator
Copy link
Author

It seems the following pattern may be part of the root cause.

line 208 of server.ts file:
// Invoke the handler for the method requested const handler = this.handlers[req.method as keyof Handlers] if (handler) { return handler.send(req, res).catch((error) => { log([${handler.constructor.name}], error) const status_code = error.status_code || ERRORS.UNKNOWN_ERROR.status_code const body = error.body || ${ERRORS.UNKNOWN_ERROR.body}${error.message || ''}\n return handler.write(res, status_code, {}, body) }) }

the hanlder.send writes the file the store and calls:

const writtenRes = this.write(res, 201, {Location: url, ...headers})
or
const writtenRes = this.write(res, 204, headers)

the write method, line 25, from the basehandler.ts:

write(res: http.ServerResponse, status: number, headers = {}, body = '') { headers = status === 204 ? headers : {...headers, 'Content-Length': body.length} res.writeHead(status, headers) res.write(body) return res.end() }

writing the file to the store returns the entire http response to the tus client. the server runs the onUploadFinish and tries to send the error 500, but at that point i think the client has disco'd the connection already.

you should be able to see this if you add something like the following to BaseHandler.js file in your node_modules dist:

console.log('basehandler->write', status, headers, body);

you'll see the server writing both responses to the wire. Wireshark / tcpdump should confirm the tus client disconnecting after the first 201 response.

thoughts ?

@Murderlon
Copy link
Member

Thanks for looking into this. I'm currently on holiday until 22 February. I'll look into this when I'm back.

@Murderlon
Copy link
Member

Alright I looked into it. The behaviour is in fact correctly working but tus-js-client will only show it failed if you return HTTP 400.

Started a discussion here: tus/tus-js-client#557

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants