Skip to content

Conversation

@darrenjaneczek
Copy link

@darrenjaneczek darrenjaneczek commented Nov 18, 2025

What this PR does / why we need it:

To collect information about requests that yield Error.
If available, collect information from the response as well.

If the response is nil, a fake response is constructed from the error object.
Here is an example extracted from an unexpected EOF error:

"response": {
  "status": 0,
  "statusText": "ERROR",
  "httpVersion": "",
  "cookies": [],
  "headers": [],
  "content": {
    "size": 3,
    "mimeType": "",
    "text": "EOF"
  },
  "redirectURL": "",
  "headersSize": -1,
  "bodySize": 3
},
"cache": {
  "comment": "Not cached"
},
"timings": {
  "send": 0,
  "wait": 0,
  "receive": 0
},

@darrenjaneczek
Copy link
Author

Currently, this does not work well for nil responses.

@darrenjaneczek darrenjaneczek marked this pull request as draft November 18, 2025 18:50
@grafana-plugins-platform-bot grafana-plugins-platform-bot bot moved this from 📬 Triage to 🔬 In review in Plugins Platform / Grafana Community Nov 18, 2025
// only process a response that is not nil
res = f.processResponse(originalRes)
}
defer res.Body.Close()
Copy link
Member

Choose a reason for hiding this comment

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

will this defer need a nil check?

Copy link
Author

Choose a reason for hiding this comment

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

Fortunately no. This is old code that I will revert. It is no longer expected to be nil: Callers of this method now send a fake response that encapsulates the error if no response was created.

if !skipSaving {
// save information from the request and failed response (if anything is available)
if res == nil {
res = &http.Response{
Copy link
Author

@darrenjaneczek darrenjaneczek Nov 18, 2025

Choose a reason for hiding this comment

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

This might have some unexpected consequences.
We are overwriting a nil res, which will later be returned. Perhaps a different variable name.

resToAdd := res

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🔬 In review

Development

Successfully merging this pull request may close these issues.

2 participants