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

"Store files in custom nested directories" example fails with can't patch error #619

Closed
KirioXX opened this issue May 13, 2024 · 10 comments
Closed

Comments

@KirioXX
Copy link

KirioXX commented May 13, 2024

Hi I try to implement the the nested directory example,
but when I try to upload via the uppy tus service I get a can't patch error.

Error: tus: unexpected response while uploading chunk, originated from request (method: PATCH, url: http://127.0.0.1:1080/files/dXNlcnMvdGVzdC9jMjZkYWQyNjIwOGQ4Y2I1Y2JiNzQ0NWU1NGY2MmQzMA, response code: 404, response text: <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Cannot PATCH /files/dXNlcnMvdGVzdC9jMjZkYWQyNjIwOGQ4Y2I1Y2JiNzQ0NWU1NGY2MmQzMA</pre>
</body>
</html>
, request id: n/a)
    at Upload2._emitHttpError (@uppy_tus.js?v=64ccd693:1508:23)
    at @uppy_tus.js?v=64ccd693:1728:18

This is my test server:

const { Server } = require('@tus/server');
const { FileStore } = require('@tus/file-store');
const express = require('express');
const cors = require('cors');
const crypto = require('crypto');

const path = '/files';
const server = new Server({
  path,
  datastore: new FileStore({ directory: './files' }),
  namingFunction(req) {
    const id = crypto.randomBytes(16).toString('hex');
    const folder = 'test'; // your custom logic
    return `users/${folder}/${id}`;
  },
  generateUrl(_, { proto, host, path, id }) {
    id = Buffer.from(id, 'utf-8').toString('base64url');
    return `${proto}://${host}${path}/${id}`;
  },
  getFileIdFromRequest(req) {
    const reExtractFileID = /([^/]+)\/?$/;
    const match = reExtractFileID.exec(req.url);

    if (!match || path.includes(match[1])) {
      return;
    }

    return Buffer.from(match[1], 'base64url').toString('utf-8');
  },
});

const host = '127.0.0.1';
const port = 1080;
const app = express();
const uploadApp = express();
app.use(cors());

uploadApp.all('*', server.handle.bind(server));
app.use('/uploads', uploadApp);
app.listen(port, host);

and this is my very basic uppy setup:

import {Dashboard} from '@uppy/react'
import Uppy from '@uppy/core'
import Tus from '@uppy/tus'

import '@uppy/core/dist/style.min.css';
import '@uppy/dashboard/dist/style.min.css';

const endpoint = 'http://127.0.0.1:1080/uploads'

function App() {
  const uppy = new Uppy({
    debug: true,
    logger: {
      debug: console.log,
      info: console.info,
      warn: console.warn,
      error: console.error
    },
  }).use(Tus, {
    endpoint,
  });

  return (
    <Dashboard
      uppy={uppy}
      width='100vw'
      height='calc(100vh - 50px)'
      proudlyDisplayPoweredByUppy={false}
    />
  )
}

export default App

Is there any think that I could do wrong that could cause the upload to fail?

@Murderlon
Copy link
Member

Hi, might have to do that you are using Uppy in React incorrectly. Uppy can't be declared as a simple variable inside your component. Checkout the docs: https://uppy.io/docs/react/.

@KirioXX
Copy link
Author

KirioXX commented May 14, 2024

Hi @Murderlon, thanks for the quick response.
I tried to initialise uppy in a useState but I still get the same error.
This is the new component:

import {Dashboard} from '@uppy/react'
import Uppy from '@uppy/core'
import Tus from '@uppy/tus'
import React, {useState} from 'react'

import '@uppy/core/dist/style.min.css';
import '@uppy/dashboard/dist/style.min.css';

const endpoint = 'http://127.0.0.1:1080/uploads'

function App() {
  const [uppy] = useState(() => new Uppy({
    debug: true,
    logger: {
      debug: console.log,
      info: console.info,
      warn: console.warn,
      error: console.error
    },
  }).use(Tus, {
    endpoint,
  }));

  return (
    <Dashboard
      uppy={uppy}
      width='100vw'
      height='calc(100vh - 50px)'
      proudlyDisplayPoweredByUppy={false}
    />
  )
}

export default App

I'm not sure if I understand the generateUrl and getFileIdFromRequest fully,
could it be that the mapping to the file is wrong in one of those functions?

@Murderlon
Copy link
Member

I'll see if I can reproduce with a test

@Murderlon
Copy link
Member

I know this works for Supabase, as can be seen here:
https://github.com/supabase/storage/blob/30351a147e2a5d8d4ca0036c2507597793b78c9c/src/http/routes/tus/lifecycle.ts#L81-L143

Maybe that serves as inspiration in the meantime.

@Murderlon
Copy link
Member

Murderlon commented May 14, 2024

I actually already wrote a test for exactly this once:

it('can use namingFunction to create a nested directory structure', (done) => {
const route = '/test/output'
const server = new Server({
path: route,
datastore: new FileStore({directory}),
namingFunction() {
return `foo/bar/id`
},
generateUrl(_, {proto, host, path, id}) {
id = Buffer.from(id, 'utf-8').toString('base64url')
return `${proto}://${host}${path}/${id}`
},
getFileIdFromRequest(req) {
const reExtractFileID = /([^/]+)\/?$/
const match = reExtractFileID.exec(req.url as string)
if (!match || route.includes(match[1])) {
return
}
return Buffer.from(match[1], 'base64url').toString('utf-8')
},
})
const length = Buffer.byteLength('test', 'utf8').toString()
const s = server.listen()
request(s)
.post(server.options.path)
.set('Tus-Resumable', TUS_RESUMABLE)
.set('Upload-Length', length)
.then((res) => {
request(s)
.patch(removeProtocol(res.headers.location))
.send('test')
.set('Tus-Resumable', TUS_RESUMABLE)
.set('Upload-Offset', '0')
.set('Content-Type', 'application/offset+octet-stream')
.expect(204, () => {
s.close()
done()
})
})
})
})

Maybe remove cors from your setup?

@KirioXX
Copy link
Author

KirioXX commented May 15, 2024

Removing the cors middleware will result in cors errors on the client.
I'm not sure why but I have the feeling that the path mapping is not correct when the request comes in.

I will try to copy the supabase setup and see if that makes a difference.
Thanks Murderlon

@Murderlon
Copy link
Member

Supabase has a multi-tenant setup so I wouldn't exactly use that. I think your problem is that you use /uploads on the client but /files on the server.

@KirioXX
Copy link
Author

KirioXX commented May 16, 2024

That is interesting, does that mean the tus server path has to match the express server path?
Because the express is setting the uploadApp to the /uploads endpoint.

@Murderlon
Copy link
Member

It should yes, as can be seen from the example.

@KirioXX
Copy link
Author

KirioXX commented May 16, 2024

I completely missed that. Thank you very much @Murderlon! 🙌

@KirioXX KirioXX closed this as completed May 16, 2024
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