mickhansen / graphql-sequelize

GraphQL & Relay for MySQL & Postgres via Sequelize
MIT License
1.9k stars 172 forks source link

dataloader-sequelize breaks context.logging? #509

Closed hamza-openinvest closed 6 years ago

hamza-openinvest commented 7 years ago

I found a bug with passing in a logging function to the graphql context, that as far as I can tell is caused by how dataloader-sequelize is used.

Minimal Repro: dataloader-sequelize-bug.zip

npm install and npm start the zip above to run server that will repro the problem. Then go to: http://localhost:4321/graphql and run the following graphql query, twice:

{
  Parent { 
    Id
    Name
    Children {
      Id
      Name
    }
  }
}

This is the expected output in console (line comments added for this issue):

// output from first query:
counter expected 1
test actual { counter: 1,
  reqCounter: 1,
  sql: 'Executing (default): SELECT `Id`, `Name` FROM `Parent` AS `Parent` LIMIT 1;' }
test actual { counter: 1,
  reqCounter: 1,
  sql: 'Executing (default): SELECT `Id`, `ParentId`, `Name` FROM `Child` AS `Child` WHERE `Child`.`ParentId` IN (1) ORDER BY `Child`.`Id` ASC;' }
// output from second query:
counter expected 2
test actual { counter: 2,
  reqCounter: 2,
  sql: 'Executing (default): SELECT `Id`, `Name` FROM `Parent` AS `Parent` LIMIT 1;' }
test actual { counter: 2,
  reqCounter: 2,
  sql: 'Executing (default): SELECT `Id`, `ParentId`, `Name` FROM `Child` AS `Child` WHERE `Child`.`ParentId` IN (1) ORDER BY `Child`.`Id` ASC;' }

Here is the actual output (line comments added for this issue):

// output from first query:
counter expected 1
test actual { counter: 1,
  reqCounter: 1,
  sql: 'Executing (default): SELECT `Id`, `Name` FROM `Parent` AS `Parent` LIMIT 1;' }
test actual { counter: 1,
  reqCounter: 1,
  sql: 'Executing (default): SELECT `Id`, `ParentId`, `Name` FROM `Child` AS `Child` WHERE `Child`.`ParentId` IN (1) ORDER BY `Child`.`Id` ASC;' }
// output from second query:
counter expected 2
test actual { counter: 2,
  reqCounter: 2,
  sql: 'Executing (default): SELECT `Id`, `Name` FROM `Parent` AS `Parent` LIMIT 1;' }
test actual { counter: 2,
  reqCounter: 1,           // <----- wrong!
  sql: 'Executing (default): SELECT `Id`, `ParentId`, `Name` FROM `Child` AS `Child` WHERE `Child`.`ParentId` IN (1) ORDER BY `Child`.`Id` ASC;' }

I'm not sure if the problem is with how dataloader-sequelize is used, but I do get the expected output if I comment out this line: https://github.com/mickhansen/graphql-sequelize/blob/master/src/resolver.js#L20

I think this could also lead to some serious security problems, e.g. the passport library adds a user object to the express request object, so if consequent graphql requests get the old request object the authorization will be for a different user!

Thanks for the library, and in advance for your help with this issue!

hamza-openinvest commented 7 years ago

Actually, this seems to refer to other symptoms from the same cause: https://github.com/mickhansen/dataloader-sequelize/issues/27 Is that correct?

mickhansen commented 7 years ago

It might very well be the same cause.

hamza-openinvest commented 6 years ago

@mickhansen was this closed because #567 fixed it?