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

GraphQL Tester Can't Handle Redirects #19

Open
machineghost opened this issue Jan 4, 2018 · 0 comments
Open

GraphQL Tester Can't Handle Redirects #19

machineghost opened this issue Jan 4, 2018 · 0 comments

Comments

@machineghost
Copy link

machineghost commented Jan 4, 2018

I have a GraphQL endpoint that requires authentication. I protect with some very simple (passport-based) middleware:

const redirectIfNotAuthenticated = (request, response, next) =>
  request.isAuthenticated() && request.session.user
    ? next()
    : response.redirect('/login');

// ...
app.use('/graphql', redirectIfNotAuthenticated, graphqlHTTP(request => ({ //...

This works great ... but not so great for testing. When GraphQL Tester gets that redirect response ('Found. Redirecting to /login') it tries to parse it like JSON:

// main/index.js
if (error) {
  reject(error);
} else {
  var result = JSON.parse(body); // body isn't JSON, its a redirect
  resolve({ //...

It'd be really great if the library could detect redirects and resolve with its details, but if that doesn't fit the library then at the least it should reject the promise with the proper details (instead of throwing an un-catachable exception).

In other words, instead of the current behavior of throwing an error:

SyntaxError: Unexpected token F in JSON at position 0
    at JSON.parse (<anonymous>)
    at Request._callback (server/node_modules/graphql-tester/lib/main/index.js:72:43)
    at Request.self.callback (server/node_modules/request/request.js:186:22)
    at Request.<anonymous> (server/node_modules/request/request.js:1163:10)
    at IncomingMessage.<anonymous> (server/node_modules/request/request.js:1085:12)
    at endReadableNT (_stream_readable.js:1056:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickDomainCallback (internal/process/next_tick.js:218:9)

it would be better if it did something like:

} else {
  try {
    preValidateThatBodyIsJSON(body);
    var result = JSON.parse(body); 
    resolve({ //...
 } catch (parseError) {
   reject(parseError);
 }

(where preValidateThatBodyIsJSON throws a catch-able error if the body is not JSON)

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

1 participant