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 panics on non-GET/non-POST requests #186

Merged
merged 3 commits into from
Mar 20, 2023

Conversation

JohnStarich
Copy link
Member

Stack trace from the original panic:

       panic: runtime error: index out of range [0] with length 0

goroutine 20 [running]:
testing.tRunner.func1.2({0x102910300, 0x140001682e8})
        src/testing/testing.go:1396 +0x1c8
testing.tRunner.func1()
        src/testing/testing.go:1399 +0x378
panic({0x102910300, 0x140001682e8})
        src/runtime/panic.go:884 +0x204
github.com/nautilus/gateway.(*Gateway).GraphQLHandler(0x1400015a100, {0x10293e208, 0x14000163540}, 0x1400015a300)
        gateway/http.go:150 +0x8c4
github.com/nautilus/gateway.TestGraphQLHandler_OptionsMethod(0x140001029c0)
        gateway/http_test.go:1470 +0x250
testing.tRunner(0x140001029c0, 0x102937a80)
        src/testing/testing.go:1446 +0x10c
created by testing.(*T).Run
        src/testing/testing.go:1493 +0x300
FAIL    github.com/nautilus/gateway     0.778s
FAIL

It failed to form a response because none had been created. This PR prevents further execution when the HTTP method is not supported.

The GraphQL over HTTP working draft provides a little bit of clarity here, though stops short of defining non-GET and non-POST response status codes.

Given most servers return 405 Method Not Allowed in these kinds of scenarios and it is certainly better than panicking (typically with a 500 error), I think this is the right move for now.

// the error we have encountered when extracting query input

// make our lives easier. track if we're in batch mode
batchMode = false
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this line since it is already false by default when not set. (Go's zero-value)

@JohnStarich JohnStarich merged commit a825e2e into master Mar 20, 2023
@JohnStarich JohnStarich deleted the bugfix/handle-non-get-post-methods branch March 20, 2023 20:50
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

1 participant