Skip to content

Refactor REST client #286

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

Merged
merged 5 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions src/api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Api } from "coder/site/src/api/api"
import fs from "fs/promises"
import * as https from "https"
import * as os from "os"
import * as vscode from "vscode"
import { CertificateError } from "./error"
import { Storage } from "./storage"

// expandPath will expand ${userHome} in the input string.
const expandPath = (input: string): string => {
const userHome = os.homedir()
return input.replace(/\${userHome}/g, userHome)
}

/**
* Create an sdk instance using the provided URL and token and hook it up to
* configuration. The token may be undefined if some other form of
* authentication is being used.
*/
export async function makeCoderSdk(baseUrl: string, token: string | undefined, storage: Storage): Promise<Api> {
const restClient = new Api()
restClient.setHost(baseUrl)
if (token) {
restClient.setSessionToken(token)
}

restClient.getAxiosInstance().interceptors.request.use(async (config) => {
Copy link
Member

Choose a reason for hiding this comment

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

Not a huge deal, but is there a reason why the Axios instance isn't a separate variable?

Copy link
Member

Choose a reason for hiding this comment

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

Though I guess another option would be to add both interceptors in one call of interceptors.request.use

Copy link
Member Author

@code-asher code-asher May 30, 2024

Choose a reason for hiding this comment

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

Oh like why not split out into const axios = restClient.getAxiosInstance() type of thing?

Could do that, I tend to like leaving things on their objects and accessing them that way, mostly just a preference thing I think. I think maybe I like having them "namespaced" or something like that.

Edit: to add that this only applies to simple getters, if the function does work or the return value can change I would encapsulate into a var for sure

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh and one is a request interceptor and the other a response interceptor so I think they have to be separate calls

Copy link
Member

Choose a reason for hiding this comment

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

Oh, wait – you're right. Totally missed that they were different names

// Add headers from the header command.
Object.entries(await storage.getHeaders(baseUrl)).forEach(([key, value]) => {
config.headers[key] = value
})

// Configure TLS.
const cfg = vscode.workspace.getConfiguration()
const insecure = Boolean(cfg.get("coder.insecure"))
const certFile = expandPath(String(cfg.get("coder.tlsCertFile") ?? "").trim())
const keyFile = expandPath(String(cfg.get("coder.tlsKeyFile") ?? "").trim())
const caFile = expandPath(String(cfg.get("coder.tlsCaFile") ?? "").trim())

config.httpsAgent = new https.Agent({
cert: certFile === "" ? undefined : await fs.readFile(certFile),
key: keyFile === "" ? undefined : await fs.readFile(keyFile),
ca: caFile === "" ? undefined : await fs.readFile(caFile),
// rejectUnauthorized defaults to true, so we need to explicitly set it to false
// if we want to allow self-signed certificates.
rejectUnauthorized: !insecure,
})

return config
})

// Wrap certificate errors.
restClient.getAxiosInstance().interceptors.response.use(
(r) => r,
async (err) => {
throw await CertificateError.maybeWrap(err, baseUrl, storage)
},
)

return restClient
}
Loading