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

Fix startup crash when a non-superuser tries to watch #371

Merged
merged 3 commits into from
Mar 5, 2017

Conversation

MaienM
Copy link

@MaienM MaienM commented Feb 22, 2017

The issue was that the transaction failed but was not rolled back, causing all subsequent commands to fail. This just adds a rollback in the catch, and it adjusts the tests to expect this.

Fixes #368, replaces #369.

Copy link
Collaborator

@calebmer calebmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s cool that this works 👍

I wouldn’t expect rollback to work outside of a transaction.

@calebmer calebmer merged commit 6b3374b into graphile:master Mar 5, 2017
@benjie
Copy link
Member

benjie commented Mar 5, 2017

What voodoo is this?

Issuing ROLLBACK outside of a transaction block emits a warning and otherwise has no effect.
-- https://www.postgresql.org/docs/9.6/static/sql-rollback.html

@benjie
Copy link
Member

benjie commented Mar 5, 2017

Ah; the query is from:

https://github.com/calebmer/postgraphql/blob/6b3374baa974d8efd110970e4bd9431b8b3771f3/resources/watch-fixtures.sql

This query issues a begin; it seems that the error thrown prevents further statements being executed. This is unexpected to me... I would expect pgClient to execute all of the statements issued, even if errors occur, ultimately culminating in the commit; (which would perform an rollback due to the errors) and then return the results (i.e. throw the error).

@MaienM MaienM deleted the issue-368 branch March 5, 2017 17:51
Belline pushed a commit to Belline/postgraphql that referenced this pull request Dec 18, 2017
…aphile#371)

* fix(watch): fix startup crash when non-superuser tries to watch

* Update watchPgSchemas.ts
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

Successfully merging this pull request may close these issues.

None yet

3 participants