Unexpected dead of HTTP request for completion endpoint

[Nexucis]

Using the following request will killed the HTTP request on the server side:

curl -XPOST http://localhost:8080/completion -d '{"expr":"sum ", "limit": 100, "positionChar": 4, "positionLine": 0}'

curl: (52) Empty reply from server

On server side

2020/07/17 17:37:03 http: panic serving [::1]:51207: runtime error: invalid memory address or nil pointer dereference
goroutine 13 [running]:
net/http.(*conn).serve.func1(0xc0002163c0)
        C:/Go/src/net/http/server.go:1772 +0x140
panic(0x9ef5a0, 0xfa65e0)
        C:/Go/src/runtime/panic.go:975 +0x3f1
github.com/prometheus-community/promql-langserver/rest.(*API).completion(0xc000308510, 0xb7ad80, 0xc00034c1c0, 0xc000356600)
        E:/workspace/go/src/github.com/prometheus-community/promql-langserver/rest/api.go:207 +0x181
github.com/prometheus-community/promql-langserver/rest.manageDocumentMiddleware.func1.1(0xb7ad80, 0xc00034c1c0, 0xc000356400)
        E:/workspace/go/src/github.com/prometheus-community/promql-langserver/rest/middleware.go:130 +0x79c
github.com/prometheus-community/promql-langserver/rest.(*apiMetrics).instrumentHTTPRequest.func1(0xb7ad80, 0xc00034c1c0, 0xc000356400)
        E:/workspace/go/src/github.com/prometheus-community/promql-langserver/rest/middleware.go:175 +0xa0

[Nexucis]

I supposed the issue is on all endpoint. It's just because the error returned is nil and the struct is nil too.

[slrtbtfs]

Ouch, nice find!

---

Returning nil without an error if there is nothing to report is expected behaviour for the language server. So the bug is in the REST package.

More concretely, nil pointer checks are missing in two places:

promql-langserver/rest/api.go

Line 143 in 4e7ee5f

items := diagnostics.Diagnostics

promql-langserver/rest/api.go

Line 207 in 4e7ee5f

items := completion.Items

However, for this particular example request, fixing #185 would probably have prevented the crash as well, since the invalid position should cause a cache access error.

[Nexucis]

if there is no result, it would be nice to return an empty struct instead don't you think ?

We would avoid this kind of situation at least

[slrtbtfs]

It wouldn't hurt to do that and is the nicer solution.

[slrtbtfs]

if there is no result, it would be nice to return an empty struct instead don't you think ?

On second thought, that's not always what we want. For example in the case of a signature request with a cursor outside a function call, returning a non-nil value would suggest, the language server actually computed a signature.