apollo-server-integrations / apollo-server-integration-hapi

MIT License
3 stars 3 forks source link

problem handling preflight 'OPTIONS' reqeusts #18

Closed deanshmuel closed 1 year ago

deanshmuel commented 1 year ago

Hey It seems that the plugin forwards 'OPTIONS' preflight requests to apollo-server, I'm getting the error "Apollo Server supports only GET/POST requests.". I'm using "@as-integrations/hapi": "^1.0.1", with "@apollo/server": "^4.2.2", My server is "@hapi/hapi": "^21.1.0", I'm registering the plugin on the hapi sever like this: server.register({ plugin: require('@as-integrations/hapi'), options: { apolloServer, context:, postRoute: { options: { cors: { origin: ['*'], } } }, getRoute: { options: { cors: { origin: ['*'], } } } } });

Browser requests fails with the error mentioned above. When I'm deleting the route for OPTIONS defined by the plugin everything works well.

What am I doing wrong?

Thanks a lot! Dean

arimus commented 1 year ago

@deanshmuel Can you provide a working example of this problem and how you tested? The options paths were configured to specifically handle CORS requests that were failing without it.

Apollo CORS docs describe how Apollo Server responds to OPTIONS requests, so I'm not sure why you would get this. @trevor-scheer may know more about the behavior and CORS support in the Apollo Server Core.

https://www.apollographql.com/docs/apollo-server/security/cors/

trevor-scheer commented 1 year ago

Apollo Server itself does not handle CORS (though our standalone server does and our documentation for the express middleware describes how to install the CORS middleware).

Ultimately, an OPTIONS request should not be passed to Apollo Server. Either the plugin needs to be responsible for handling CORS or it should be documented how users are expected to configure CORS within their Hapi instance so that OPTIONS requests are handled correctly.

arimus commented 1 year ago

Makes sense. I may need to make some adjustments then. Thanks @trevor-scheer

arimus commented 1 year ago

I've just pushed v1.1.1 which has default CORS enabled, but which can also be overridden. @deanshmuel give the new version a whack and let me know if this resolves your issue

terrisgit commented 1 year ago

This is unrelated but I got here because this is the only strong hit on the search require('@as-integrations/hapi'). If you are trying to use @as-integrations/hapi with CommonJS, require('@as-integrations/hapi') doesn't work. Use this instead: plugin: require('@as-integrations/hapi').default

deanshmuel commented 1 year ago

Hi sorry for the late response. I eventually solved the issue by cloning the repo and deleting the route for 'OPTIONS' it sets on the Hapi-server. I handle all of the cors request straight on the server as documented in Hapi docs

arimus commented 1 year ago

@terrisgit There is a default export for the package. Do you happen to have a test project you could share that I can use to see what you are seeing? There are a number of configurations that could be causing the issue, including babel setup, etc.

arimus commented 1 year ago

@deanshmuel you definitely shouldn't have to clone and patch for the options settings. Did you ever try v1.1.1, which has the fix for CORS overriding? If that's not working for you, I'd like to find the issue and get it fixed. Thanks!

terrisgit commented 1 year ago

@arimus I'm using commonjs - in other words there is no "type: module" in package.json. It's an old project that has hundreds of require statements.

arimus commented 1 year ago

@terrisgit got it. Yeah, depending on your setup you may have to do exactly what you are doing now. Just for reference, here's a thread about default exports and babel. Some build systems have monkey-patched to make it cleaner on the importing side. Not sure that I can easily do anything that won't have downstream consequences, but I'll look into it some more.

https://stackoverflow.com/questions/33704714/cant-require-default-export-value-in-babel-6-x

I'll create a new issue for this to track and document. Keeping this thread open for @deanshmuel to get back on the OPTIONS issue

terrisgit commented 1 year ago

By the way, 1.1.1 and cors are working fine for me, and I need it

deanshmuel commented 1 year ago

@arimus you can definitely close the issue, I bumped the version to 1.1.1 and it works pretty well. Thanks a lot!

arimus commented 1 year ago

Thanks @deanshmuel, appreciate the feedback!