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

With runtime typeError a statusCode of 200 is surfaced #469

Closed
samueljoli opened this issue Jul 19, 2017 · 2 comments
Closed

With runtime typeError a statusCode of 200 is surfaced #469

samueljoli opened this issue Jul 19, 2017 · 2 comments

Comments

@samueljoli
Copy link

Versions

  • Hapi - 16.x.x
  • graphql-server-hapi - 1.x.x
  • Node - v6.5.0

Issue
Unless I add the config option of formatResponse if I encounter a typeError in my internal server logic I will get back a statusCode of 200. When I add formatResponse a 500 will surface as expected.

Reproduce Error

  • Deliberate TypeError, statusCode of 200 is sent back to Client
server.register({
    register: graphqlHapi,
    options : {
        path          : '/graphql',
        graphqlOptions: {
            schema   : schema,
            graphiql : true,
            rootValue: server.methods,
            context: { name: 'Samuel Joli' }
        }
    }
});

const fetchDataFunction = {}
fetchDataFunction.get = function() { /* Fetch some data */ } // Deliberate incorrect namespacing

const query = new graphql.GraphQLObjectType({
    name  : 'Query',
    fields: {
        storefront: {
            type: Storefront,
            args: {
                id: { type: graphql.GraphQLString }
            },
            resolve(_, {id}) {
                return fetchDataFunction.es.get({ // function does not exist on 'get' object but on 'es'
                    dataSource: 'marketplace.storefronts',
                    id
                }).then((response) => {
                    const doc = response.hits[0];

                    return _.storefronts.map(doc, true, true);
                })
                .catch((err) => {
                    throw new Error(Boom.wrap(err));
                })
            }
        }
    }
});
  • Deliberate TypeError, statusCode of 500 is sent back to Client
server.register({
    register: graphqlHapi,
    options : {
        path          : '/graphql',
        graphqlOptions: {
            schema   : schema,
            graphiql : true,
            rootValue: server.methods,
            formatResponse: function() {
            },
            context: { name: 'Samuel Joli' }
        }
    }
});

const fetchDataFunction = {}
fetchDataFunction.get = function() { /* Fetch some data */ } // Deliberate incorrect namespacing

const query = new graphql.GraphQLObjectType({
    name  : 'Query',
    fields: {
        storefront: {
            type: Storefront,
            args: {
                id: { type: graphql.GraphQLString }
            },
            resolve(_, {id}) {
                return fetchDataFunction.es.get({ // function does not exist on 'get' object but on 'es'
                    dataSource: 'marketplace.storefronts',
                    id
                }).then((response) => {
                    const doc = response.hits[0];

                    return _.storefronts.map(doc, true, true);
                })
                .catch((err) => {
                    throw new Error(Boom.wrap(err));
                })
            }
        }
    }
});
@freiksenet
Copy link
Contributor

Thank you for the report!

GraphQL servers don't typically return HTTP response codes in case of failure, they return the errors as errors field in the response. So not returning 500 is an expected behaviour.

However, there seems to be a bug in the lib that causes the server to crash when the formatResponse returns undefined, like in the second case here. I've added an issue for this #487.

@samueljoli
Copy link
Author

@freiksenet Ahhh Yeah I've definitely read up some more on the technology that is graphql and it's definitely a shift in thinking when it comes to api development. I'll look into issue 487. Also do you guys/gals think it would be helpful if we introduced framework specific labels?

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

No branches or pull requests

2 participants