Skip to content

Commit

Permalink
Fix diagnose report's Push API key transmission (#474)
Browse files Browse the repository at this point in the history
## The problem

The diagnose report was transmitted without the Push API key as the
`api_key` query parameter. This caused the reports to not be properly
linked to their parent organizations on AppSignal.com, making it more
difficult to find them back for users.

It also caused the issues described in this server side issue:
appsignal/appsignal-server#7710

## The solution

The request to submit the report now includes the `api_key` query
parameter.

I've moved the report transmission to the DiagnoseTool, because the
AppSignal config is available there, and I didn't want to initialize it
again in the Diagnose CLI class or move it to the Diagnose CLI class, or
fetch it from the Diagnose Tool's generated report. There's no big
design idea behind it, this was just easiest.

I've implemented error handling when the server returned an error code,
because otherwise it would print an ugly backtrace with where the JSON
parsing failed.

This behavior's tests should be implemented in the diagnose tests
submodule when we test the transmitted report as well.
  • Loading branch information
tombruijn authored Nov 2, 2021
1 parent ea68529 commit 1d3eccc
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 42 deletions.
5 changes: 5 additions & 0 deletions packages/nodejs/.changesets/diagnose-api-key-transmission.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
bump: "patch"
---

Fix diagnose report recognition when sent to the server. It was sent without an `api_key` parameter, which resulted in apps not being linked to the parent organization based on the known Push API key.
51 changes: 9 additions & 42 deletions packages/nodejs/src/cli/diagnose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@ const readline = require("readline")
import { HashMap } from "@appsignal/types"

export class Diagnose {
#diagnose: typeof DiagnoseTool

constructor() {
this.#diagnose = new DiagnoseTool({})
}

public async run() {
const data = await new DiagnoseTool({}).generate()
const data = await this.#diagnose.generate()

console.log(`AppSignal diagnose`)
console.log(`=`.repeat(80))
Expand Down Expand Up @@ -218,7 +224,7 @@ export class Diagnose {
` Not sending report. (Specified with the --no-send-report option.)`
)
} else if (process.argv.includes("--send-report")) {
this.send_report(data)
this.#diagnose.sendReport(data)
} else {
const rl = readline.createInterface({
input: process.stdin,
Expand All @@ -231,7 +237,7 @@ export class Diagnose {
function (answer: String) {
switch (answer || "y") {
case "y":
self.send_report(data)
self.#diagnose.sendReport(data)
break

default:
Expand Down Expand Up @@ -348,45 +354,6 @@ export class Diagnose {
}
}

send_report(data: object) {
const json = JSON.stringify(data)

const opts = {
port: 443,
method: "POST",
host: "appsignal.com",
path: "/diag",
headers: {
"Content-Type": "application/json",
"Content-Length": json.length
},
cert: fs.readFileSync(
path.resolve(__dirname, "../../cert/cacert.pem"),
"utf-8"
)
}

const req = https.request(opts, (res: any) => {
res.setEncoding("utf8")

// print token to the console
res.on("data", (chunk: any) => {
const { token } = JSON.parse(chunk.toString())
console.log(` Your support token:`, token)
console.log(
` View this report: https://appsignal.com/diagnose/${token}`
)
})
})

req.on("error", (e: any) => {
console.error(`Problem with diagnose request: ${e.message}`)
})

req.write(json)
req.end()
}

print_newline() {
console.log(``)
}
Expand Down
46 changes: 46 additions & 0 deletions packages/nodejs/src/diagnose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,52 @@ export class DiagnoseTool {

return config
}

public sendReport(data: object) {
const json = JSON.stringify(data)

const config = this.#config.data
const params = new URLSearchParams({ api_key: config["apiKey"] || "" })

const opts = {
port: 443,
method: "POST",
host: "appsignal.com",
path: `/diag?${params.toString()}`,
headers: {
"Content-Type": "application/json",
"Content-Length": json.length
},
cert: fs.readFileSync(
path.resolve(__dirname, "../cert/cacert.pem"),
"utf-8"
)
}

const request = https.request(opts, (response: any) => {
const responseStatus = response.statusCode
response.setEncoding("utf8")

response.on("data", (responseData: any) => {
if (responseStatus === 200) {
const { token } = JSON.parse(responseData.toString())
console.log(` Your support token:`, token)
console.log(
` View this report: https://appsignal.com/diagnose/${token}`
)
} else {
console.error(
" Error: Something went wrong while submitting the report to AppSignal."
)
console.error(` Response code: ${responseStatus}`)
console.error(` Response body:\n${responseData}`)
}
})
})

request.write(json)
request.end()
}
}

// This implementation should match the `packages/nodejs-ext/scripts/report.js`
Expand Down

0 comments on commit 1d3eccc

Please sign in to comment.