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

Query batching fails #22

Open
josephktcheung opened this issue May 21, 2018 · 6 comments
Open

Query batching fails #22

josephktcheung opened this issue May 21, 2018 · 6 comments

Comments

@josephktcheung
Copy link
Contributor

josephktcheung commented May 21, 2018

I'm using apollo-link-batch-http to batch our graphql request but Vesper fails to run batched mutations.

Reproduction commit: https://github.com/josephktcheung/typescript-advanced-example/tree/a773ce11bc45890720b24fe432403dbee6463e7c

Steps:

  1. Reuse the typescript-advanced-example and upgraded vesper to latest
  2. Here's my index.ts:
import { bootstrap } from "vesper";
import { PostController } from "./controller/PostController";
import { Post } from "./entity/Post";
import { CategoryController } from "./controller/CategoryController";
import { Category } from "./entity/Category";
import { PostResolver } from "./resolver/PostResolver";
import { createApolloFetch } from 'apollo-fetch';

const apolloFetch = createApolloFetch({ uri: 'http://localhost:3000/graphql' });
const request = {
    "operationName": "CreateCategory",
    "query": "mutation CreateCategory {\n  categorySave(name: \"a\") {name}}\n"
};

const run = async () => {
    const single = await apolloFetch(request);
    console.log('signle request reesult: ', JSON.stringify(single, null, 2));
    const multiple = await apolloFetch([request, request]);
    console.log('multiple requests result: ', JSON.stringify(multiple, null, 2));
}

bootstrap({
    port: 3000,
    controllers: [
        PostController,
        CategoryController
    ],
    resolvers: [
        PostResolver
    ],
    entities: [
        Post,
        Category
    ],
    schemas: [__dirname + "/schema/**/*.graphql"]
}).then(() => {
    console.log("Your app is up and running on http://localhost:3000 . " +
        "You can use Playground in development mode on http://localhost:3000/playground");
    return run();
}).catch(error => {
    console.error(error.stack ? error.stack : error);
});
  1. Console logs:
Your app is up and running on http://localhost:3000 . You can use Playground in development mode on http://localhost:3000/playground
signle request reesult:  {
  "data": {
    "categorySave": {
      "name": "a"
    }
  }
}
TypeError: this.connection.createQueryRunner is not a function
    at EntityPersistExecutor.<anonymous> (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:82:75)
    at step (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:32:23)
    at Object.next (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:13:53)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:3:12)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:76:60
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
TypeError: this.connection.createQueryRunner is not a function
    at EntityPersistExecutor.<anonymous> (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:82:75)
    at step (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:32:23)
    at Object.next (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:13:53)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:3:12)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:76:60
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
TypeError: this.connection.createQueryRunner is not a function
    at EntityPersistExecutor.<anonymous> (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:82:75)
    at step (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:32:23)
    at Object.next (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:13:53)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:3:12)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:76:60
    at <anonymous>
TypeError: this.connection.createQueryRunner is not a function
    at EntityPersistExecutor.<anonymous> (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:82:75)
    at step (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:32:23)
    at Object.next (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:13:53)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:3:12)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:76:60
    at <anonymous>
multiple requests result:  [
  {
    "data": {
      "categorySave": null
    },
    "errors": [
      {
        "message": "this.connection.createQueryRunner is not a function",
        "locations": [
          {
            "line": 2,
            "column": 3
          }
        ],
        "path": [
          "categorySave"
        ]
      }
    ]
  },
  {
    "data": {
      "categorySave": null
    },
    "errors": [
      {
        "message": "this.connection.createQueryRunner is not a function",
        "locations": [
          {
            "line": 2,
            "column": 3
          }
        ],
        "path": [
          "categorySave"
        ]
      }
    ]
  }
]
@josephktcheung
Copy link
Contributor Author

josephktcheung commented May 21, 2018

Just updated my branch to use postgres, which gives different result.

Reproduction commit: https://github.com/josephktcheung/typescript-advanced-example/tree/91ccd7bad62d87ab8c1d3b47a446814f413ddbf1

Here is updated index.ts:

import { bootstrap } from "vesper";
import { PostController } from "./controller/PostController";
import { Post } from "./entity/Post";
import { CategoryController } from "./controller/CategoryController";
import { Category } from "./entity/Category";
import { PostResolver } from "./resolver/PostResolver";
import { createApolloFetch } from 'apollo-fetch';
import { getConnection } from "typeorm";

const apolloFetch = createApolloFetch({ uri: 'http://localhost:3000/graphql' });
const generateRequest = name => ({
    "operationName": "CreateCategory",
    "query": `mutation CreateCategory {\n  categorySave(name: \"${name}\") {name}}\n`
});

const run = async () => {
    const single = await apolloFetch(generateRequest('single'));
    console.log('single request result: ', JSON.stringify(single, null, 2));
    const multiple = await apolloFetch([generateRequest('multiple 1'), generateRequest('multiple 2')]);
    console.log('multiple requests result: ', JSON.stringify(multiple, null, 2));
}

const checkPersistence = async () => {
    const categoryRepository = getConnection().manager.getRepository(Category);
    const categories = await categoryRepository.find();

    console.log('categories persisted in db: ', JSON.stringify(categories, null, 2));
}

bootstrap({
    port: 3000,
    controllers: [
        PostController,
        CategoryController
    ],
    resolvers: [
        PostResolver
    ],
    entities: [
        Post,
        Category
    ],
    schemas: [__dirname + "/schema/**/*.graphql"]
}).then(() => {
    console.log("Your app is up and running on http://localhost:3000 . " +
        "You can use Playground in development mode on http://localhost:3000/playground");
    return run();
}).then(() => {
    return checkPersistence();
})
    .catch(error => {
        console.error(error.stack ? error.stack : error);
    });

And here's console log:

Your app is up and running on http://localhost:3000 . You can use Playground in development mode on http://localhost:3000/playground
single request result:  {
  "data": {
    "categorySave": {
      "name": "single"
    }
  }
}
multiple requests result:  [
  {
    "data": {
      "categorySave": {
        "name": "multiple 1"
      }
    }
  },
  {
    "data": {
      "categorySave": {
        "name": "multiple 2"
      }
    }
  }
]
categories persisted in db:  [
  {
    "id": 1,
    "name": "single"
  }
]

You can see that this time multiple requests result shows up, but if you run a query against the database only records in single request are persisted, which is very misleading.

@josephktcheung
Copy link
Contributor Author

josephktcheung commented May 21, 2018

I hope Vesper can support query batching soon. Because I'm running Vesper on aws lambda, and with batching I can reduce number of times lambda get called, which means reducing our cost. ATM I need to set our batch size to 1 in our client ☹️

  const httpLink = new BatchHttpLink({
    uri: process.env.GRAPHQL_ENDPOINT,
    credentials: 'same-origin',
    batchMax: 1,
  });

@josephktcheung
Copy link
Contributor Author

After further investigation, the problem is more serious than I thought.

Here's the latest reproduction commit: https://github.com/josephktcheung/typescript-advanced-example/tree/817468b030572e8447fccace3da5480510dc8ef1

Here's my new index.ts:

import { bootstrap } from "vesper";
import { PostController } from "./controller/PostController";
import { Post } from "./entity/Post";
import { CategoryController } from "./controller/CategoryController";
import { Category } from "./entity/Category";
import { PostResolver } from "./resolver/PostResolver";
import { createApolloFetch } from 'apollo-fetch';
import { getConnection } from "typeorm";

const apolloFetch = createApolloFetch({ uri: 'http://localhost:3000/graphql' });
const generateRequest = name => ({
    "operationName": "CreateCategory",
    "query": `mutation CreateCategory {\n  categorySave(name: \"${name}\") {name}}\n`
});

const run = async () => {
    const single = await apolloFetch(generateRequest('single'));
    console.log('single request result: ', JSON.stringify(single, null, 2));
    const batch = await apolloFetch([generateRequest('batch 1'), generateRequest('batch 2')]);
    console.log('batch requests result: ', JSON.stringify(batch, null, 2));
    const asynchronous = await Promise.all([
        apolloFetch(generateRequest('asynchronous 1')), apolloFetch(generateRequest('asynchronous 2'))
    ]);
    console.log('asynchronous requests result: ', JSON.stringify(asynchronous, null, 2));
}


const checkPersistence = async () => {
    const categoryRepository = getConnection().manager.getRepository(Category);
    const categories = await categoryRepository.find();

    console.log('categories persisted in db: ', JSON.stringify(categories, null, 2));
}

bootstrap({
    port: 3000,
    controllers: [
        PostController,
        CategoryController
    ],
    resolvers: [
        PostResolver
    ],
    entities: [
        Post,
        Category
    ],
    schemas: [__dirname + "/schema/**/*.graphql"]
}).then(() => {
    console.log("Your app is up and running on http://localhost:3000 . " +
        "You can use Playground in development mode on http://localhost:3000/playground");
    return run();
}).then(() => {
    return checkPersistence();
})
    .catch(error => {
        console.error(error.stack ? error.stack : error);
    });

And here's console logs:

Your app is up and running on http://localhost:3000 . You can use Playground in development mode on http://localhost:3000/playground
single request result:  {
  "data": {
    "categorySave": {
      "name": "single"
    }
  }
}
batch requests result:  [
  {
    "data": {
      "categorySave": {
        "name": "batch 1"
      }
    }
  },
  {
    "data": {
      "categorySave": {
        "name": "batch 2"
      }
    }
  }
]
TypeError: this.connection.createQueryRunner is not a function
    at EntityPersistExecutor.<anonymous> (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:82:75)
    at step (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:32:23)
    at Object.next (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:13:53)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:3:12)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:76:60
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
TypeError: this.connection.createQueryRunner is not a function
    at EntityPersistExecutor.<anonymous> (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:82:75)
    at step (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:32:23)
    at Object.next (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:13:53)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:3:12)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/typeorm/persistence/EntityPersistExecutor.js:76:60
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
TypeError: Cannot read property 'type' of undefined
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/vesper/index.js:112:61
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
Error: Network request failed with status 500 - "Internal Server Error"
    at throwHttpError (/Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/apollo-fetch/dist/bundle.umd.js:43:21)
    at /Users/josephcheung/workspace/zeep/repos/typescript-advanced-example/node_modules/apollo-fetch/dist/bundle.umd.js:146:17
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

Even if I run apolloFetch with single request, if requests are run asynchronously the records are not persisted as well.

@josephktcheung
Copy link
Contributor Author

josephktcheung commented May 21, 2018

This means if the client sends multiple mutation requests to the server at the same time the server cannot handle them at all, not even a single one, no matter those requests are batched or not. This is a deal breaker to me tbh. I’ll try diving into source code to find out why.

@josephktcheung
Copy link
Contributor Author

josephktcheung commented May 21, 2018

So this is because by default transaction is true for mutation. Once I set transaction to false e.g. @Mutation({ transaction: false }) then it works

@josephktcheung
Copy link
Contributor Author

josephktcheung commented May 21, 2018

This makes me wonder if transaction should not be true by default, because setting this to true implies that mutation cannot be run simultaneously, which we don't need it in most cases unless it's critical mutation like charging customer. By making user marking transaction to true by themselves would make it clear to user which mutation is mission critical. For my projects I believe 90% of mutations don't need transaction, that means I need to manually set 90% of them to false :(

I understand that making every mutation transactional has its merits, but the current implementation makes it impossible to perform asynchronous / batch mutations if the batch includes more than 1 transactional mutation, even if 2 mutations are different e.g. postSave and photoSave will fail if you send them together to Vesper (assuming both of them are transactional mutations)

What do you think @pleerock ?

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