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

Hapi Integration #5

Closed
altschuler opened this issue May 30, 2017 · 11 comments
Closed

Hapi Integration #5

altschuler opened this issue May 30, 2017 · 11 comments

Comments

@altschuler
Copy link

It would be great to have support for hapijs. I might take a look at this if I find the time.

@jaydenseric
Copy link
Owner

I don't know much about hapi, but if you are sure the Express middleware won't work just import the processRequest function and go from there.

import {processRequest} from 'apollo-upload-server'

Once you have something working we can add it in here.

@jaydenseric
Copy link
Owner

Closing this for now, feel free to submit a pull request if you get around to working on it.

@TdyP
Copy link

TdyP commented Jun 20, 2017

I'm trying to get this working with Hapi but quickly got stuck with this error:

Error: Unknown event error
    at Object.exports.assert (/Users/teddy/leav_core/node_modules/podium/node_modules/hoek/lib/index.js:736:11)
    at internals.Request.internals.Podium.on.internals.Podium.addListener (/Users/teddy/leav_core/node_modules/podium/lib/index.js:291:10)
    at IncomingForm.parse (/Users/teddy/leav_core/node_modules/formidable/lib/incoming_form.js:126:6)
    at Promise (/Users/teddy/leav_core/node_modules/apollo-upload-server/dist/apollo-upload-server.js:25:10)
    at Promise (<anonymous>)
    at processRequest (/Users/teddy/leav_core/node_modules/apollo-upload-server/src/index.js:16:10)
    at _callee2$ (/Users/teddy/leav_core/server.js:142:68)
    at tryCatch (/Users/teddy/leav_core/node_modules/regenerator-runtime/runtime.js:65:40)
    at Generator.invoke [as _invoke] (/Users/teddy/leav_core/node_modules/regenerator-runtime/runtime.js:303:22)
    at Generator.prototype.(anonymous function) [as next] (/Users/teddy/leav_core/node_modules/regenerator-runtime/runtime.js:117:21)

I'm just calling processRequest in a prerequisiteslike that

server.register({
    register: graphqlHapi,
    options: {
        path: '/graphql',
        route: {
            pre: [
                {
                    method: async (req, reply) => {
                        try {
                            const processedRequest = await processRequest(req, {
                                uploadDir: '/tmp'
                            });
                        } catch(e) {
                            console.log(e);
                        }
                    },
                    assign: 'fileUpload'
                }
            ]
        },
        graphqlOptions: (req) => { ... }
    }
});

I don't think this error is linked to your code, it might come from formidable but I was wondering if you've seen it sometimes during the development?

@jaydenseric
Copy link
Owner

I don't really understand the way registering that middleware works, but are you sure req being passed in is the original, vanilla multipart form? Some routers have the original request nested as a property under their abstracted request.

@altschuler
Copy link
Author

With this approach you have to use req.raw.request to get the vanilla node request as @jaydenseric mentioned. I tried this too, but got stuck because Hapi will parse the payload for you before it reaches the pre handler. There might be a way around it though.

@TdyP I'm very interested in this, so please do update this thread with anything you discover :) Happy to help out as well.

@jaydenseric
Copy link
Owner

Maybe we should abstract out the logic of processRequest and export a new processOperation function, so that those who have already parsed the multipart form some other way can run the logic without formidable getting in the way.

@altschuler
Copy link
Author

That might work, but as far as I remember the problem was more that Hapi wants either multipart or regular, it's difficult to support both, and that complicated things a lot. Also, the apollo hapi plugin uses both GET and POST for the route which further complicated matters because it's not allowed (/possible) to use multipart with a GET request.

Ideally we would need to customize the apollo hapi plugin, but then that needs to be maintained with updates from apollo etc, so that doesn't seem like a viable option.

@jaydenseric jaydenseric reopened this Jun 20, 2017
@TdyP
Copy link

TdyP commented Jun 22, 2017

Not much success so far for me.

I managed to get the request body parsing working if I plug the processRequest on any hook before the onPreAuth on request lifecycle. That's not a very clean option as these hooks are for any incoming request and not bound to a specific route. Of course, I've got enough information to skip any not-graphql query, but this approach looks too hacky. Moreover, it seems to mess up with Hapi regular request processing as I receive an "Invalid multipart payload format" 400 error.

So, using the pre looks like the best option. Hapi does have an option to skip payload parsing. But, this option is not available if GET is allowed. Apollo Server set the endpoint available via GETor POST and it's hardcoded so we're stuck here too. Not sure if submitting a PR on Apollo Server to get allowed methods configurable has any chance to be accepted.

Didn't dug any further on exporting a new processOperation as this would mean handling files upload some other way, thus the whole module will become much less useful.

@altschuler
Copy link
Author

I took almost exactly the same roundtrip and got stuck. I image it would be required to either have some changes made in apollo hapi or go the super hacky way as you described.

@sallomendo
Copy link

I made it work without the apollo-upload-server package using only the Hapi route configuration, below you can see the code if it helps someone. Note that I'm not using apollo-server-hapi either.

export default {
  method: 'POST',
  path: '/graphql',
  config: {
    payload: {
      multipart: {
        output: 'file'
      }
    },
    pre: [{
      assign: 'payload',
      method ({headers, payload}, reply) {
        if (payload.operations) {
          const pay = {...JSON.parse(payload.operations)}

          each(payload, ({filename: name, headers, bytes: size, path}, key) => {
            if (name) {
              set(pay, key, {type: headers['content-type'], name, size, path})
            }
          })

          reply(pay)
        } else {
          reply()
        }
      }
    }]
  },
  async handler ({payload, pre}, reply) {
    const {query, variables} = pre.payload || payload

    try {
      const result = await graphql(schema, query, {}, {}, variables)
      reply(result)
    } catch (err) {
      reply(err)
    }
  }
}

@jaydenseric
Copy link
Owner

Closing this because it is not something I will be working on, but if anyone has an elegant solution feel free to report back or raise a PR 🙂

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

No branches or pull requests

4 participants