transloadit / uppy-server

[DEPRECATED] 'Uppy Server' was renamed to 'Companion' and lives inside the Uppy repo no
https://github.com/transloadit/uppy/tree/master/packages/%40uppy/companion
MIT License
114 stars 27 forks source link

Is this functional with Koa? #33

Closed lostpebble closed 6 years ago

lostpebble commented 7 years ago

Hi there,

Thanks for creating this awesome project, Uppy.

I was wondering if this is functional with Koa, as you have said in your docs ARCHITECTURE.md.

I see in the code it's actually just creating an express app, so I'm having my doubts. I've tried to mount the express app by passing Koa's Request and Response objects directly to it, but I'm having no luck as I would need to mount it at a sub-path, such as /_uppy-server to not interfere with my app, but it seems that there are no options for setting that.

I'm struggling to see how I can set this up on top of my existing Koa server implementation.

Acconut commented 7 years ago

I am not working on Uppy by myself but maybe this module could help you: https://github.com/vkurchatkin/koa-connect

lostpebble commented 7 years ago

Unfortunately it's not really working. It doesn't appear like uppy-server can be mounted at a sub-path within your server.

I tried with koa-connect, but it appears that it only works for express middleware - whereas this module creates an entire app for express.

I've tried this code now (using koa-mount):

function insertUppyMiddleware() {
  return mount("/_uppy-server", (ctx, next) => {
    ctx.respond = false;
    console.log(`Trying uppy-server with url: ${ctx.url}`);
    uppyExpressApp(ctx.req, ctx.res);
  });
}

Which works well at passing the trimmed url to the uppy express app. I've put in some console.log() statements and I can see the traffic is heading into the uppy-server endpoints. But I always get responses of Cannot GET /drive/connect?state=xxx for the endpoints when I try authenticate, and Uppy comes up with an error when I select Google Drive "Connection with Uppy Server Failed".

ifedapoolarewaju commented 7 years ago

@lostpebble thank you for reaching out! The Architecture.md file might have been a bit outdated. Sorry! 😞 Initially we went with the Koa route, but pending the time we deal with the challenges of supporting multiple frameworks (express, koa, hapi), we later decided to go with the most popular one for now. Apologies for the mix up 🙂

About connecting the uppy express app to your koa server, do you mind sharing with me, the configuration options, with which you are initiating the uppy app? As an example, you can see here on the standalone server, the way we are passing options to the uppy app. And you can also see the values of the options here.

lostpebble commented 7 years ago

@ifedapoolarewaju No worries :)

So, yesterday I went and double checked my configuration and it turns out that it was wrong! I had it like this:

const uppyOptions = {
  providerOptions: {
    google: {
      key: "GOOGLE_KEY",
      secret: "GOOGLE_SECRET",
    },
    server: {
      host: "localhost:3030",
      protocol: "http",
    },
    filePath: "/tmp/uppy",
  },
};

export const uppyExpressApp = uppy.app(uppyOptions);

So in actual fact I was configuring the whole thing wrong, with mistakenly putting everything under providerOptions... :sweat: Yea, I feel really stupid.

It was only after many console.log etc. I tracked down where in the code the request was being refused. And it was in the options validation check - so I went and took a closer look and to my surprise it was that all along. Unfortunately, it fails silently on options validation though so it was near impossible to know it was that which was the issue.

Another big issue was that it actually requires express-session as well. I tracked that down with my console.logs as well. In the docs it only states that body-parser is required.

In any case, I have it working alongside my Koa app now. I'm just hosting it as a completely separate server port using express itself, like so (at the end of my server config, after all the koa stuff):

const expressApp = express();

const sess = {
  secret: "uppy-sesh",
  cookie: {},
};

expressApp.use(expressSession(sess));
expressApp.use(bodyParser.json());

expressApp.use((req, res, next) => {
  res.header("Access-Control-Allow-Origin", "http://localhost:3000");
  res.header("Access-Control-Allow-Credentials", "true");
  res.header("Access-Control-Allow-Headers", "Origin, X-Requested-With, Content-Type, Accept");
  next();
});

expressApp.use("/", uppyExpressApp);

expressApp.listen("3030", () => {
  console.log("Uppy listening on port 3030");
});
ifedapoolarewaju commented 7 years ago

@lostpebble Aye! I'm glad you got this sorted out.

Unfortunately, it fails silently on options validation though so it was near impossible to know it was that which was the issue.

Because some endpoints (e.g aws s3) don't require the provider validation, the validation was made to be non-obtrusive in order to not break independent end points. But I think it would have made sense to at least log a warning, so thank you for the feedback 👍

Also express-session was initially instantiated internally, so there was no need to have this documented as a requirement. But since this had been changed, it ought to have been updated, so that's another thing added to my todo. 👍

Thank you again for hacking into this and not giving up so early 😄

Please let me know if there's anything else I can help with.