graphql api: panic when trying to lookup contents of file matches returned by zoekt #2221
Comments
ggilmore
added
bug
|
cc @slimsag. He mentioned that setting the correct revision is non-trivial since it runs the risk of harming search performance. |
ggilmore
changed the title
|
I mentioned this over Slack but:
|
|
@ijsnow @attfarhan Can you decide if this fits in the 3.1 milestone given the other work assigned to you? |
|
Would the following query work in your proposed solution (caller doesn't specifically ask for the commit)? query Search($query: String) {
search(query: $query) {
results {
results {
... on FileMatch {
file {
path
content
}
}
}
}
}
}
# Query variables
{
"query": "repo:gorilla/mux travis count:1"
}Results: |
|
Yes. As you mentioned in the description, the Effectively, that would become a call to |
|
@ijsnow I moved this to 3.2 since that's when it will ship |
|
For reference: https://github.com/sourcegraph/enterprise/issues/13258 |
Steps to reproduce:
Start the dev server with indexed search enabled:
Add http://github.com/gorilla/mux to your instance
Go the GraphQL API console (http://localhost:3080/api/console#%7B%22variables%22%3A%22%7B%5Cn%20%20%5C%22query%5C%22%3A%20%5C%22repo%3Agorilla%2Fmux%20travis%20count%3A1%5C%22%5Cn%7D%22%7D) and run the following:
Expected behavior:
The results succesfully return with the requested file contents:
{ "data": { "search": { "results": { "results": [ { "file": { "commit": { "oid": "a7962380ca08b5a188038c69871b8d3fbdf31e89" }, "path": ".travis.yml", "content": "language: go\nsudo: false\n\nmatrix:\n include:\n - go: 1.7.x\n - go: 1.8.x\n - go: 1.9.x\n - go: 1.10.x\n - go: 1.11.x\n - go: 1.x\n env: LATEST=true\n - go: tip\n allow_failures:\n - go: tip\n\ninstall:\n - # Skip\n\nscript:\n - go get -t -v ./...\n - diff -u <(echo -n) <(gofmt -d .)\n - if [[ \"$LATEST\" = true ]]; then go tool vet .; fi\n - go test -v -race ./...\n" } } ] } } } }Actual behavior:
The results have
nullcontents and contain errors:{ "data": { "search": { "results": { "results": [ { "file": { "commit": { "oid": "" }, "path": ".travis.yml", "content": null } } ] } } }, "errors": [ { "message": "graphql: panic occurred: non-absolute commit ID: \"\"", "path": [ "search", "results", "results", 0, "file", "content" ] } ] }Also see these logs:
The GraphQL results sucessfully return if you disable indexed search by adding
index:noto your search query:Query:
Results:
{ "data": { "search": { "results": { "results": [ { "file": { "commit": { "oid": "a7962380ca08b5a188038c69871b8d3fbdf31e89" }, "path": ".travis.yml", "content": "language: go\nsudo: false\n\nmatrix:\n include:\n - go: 1.7.x\n - go: 1.8.x\n - go: 1.9.x\n - go: 1.10.x\n - go: 1.11.x\n - go: 1.x\n env: LATEST=true\n - go: tip\n allow_failures:\n - go: tip\n\ninstall:\n - # Skip\n\nscript:\n - go get -t -v ./...\n - diff -u <(echo -n) <(gofmt -d .)\n - if [[ \"$LATEST\" = true ]]; then go tool vet .; fi\n - go test -v -race ./...\n" } } ] } } } }Specifiying a branch in your query e.g.
r:gorilla/mux@masteralso works.It seems like the indexed-search code path specifically provides an empty
commitfield to thefileMatchResolver:sourcegraph/cmd/frontend/graphqlbackend/textsearch.go
Line 538 in 3da30d2
However, this is an issue for the
fileMatchResolver'sContent()method:sourcegraph/cmd/frontend/graphqlbackend/file.go
Line 26 in 3da30d2
Which
panics because thecommitthat's passed to its methods isn't an absolute one:sourcegraph/pkg/vcs/git/blob.go
Line 35 in 3da30d2
The text was updated successfully, but these errors were encountered: