Skip to content

Logging adapter and usage #545

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
/coverage/
*.vsix
yarn-error.log
.DS_Store
51 changes: 51 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,54 @@
- Error handling: wrap and type errors appropriately
- Use async/await for promises, avoid explicit Promise construction where possible
- Test files must be named `*.test.ts` and use Vitest

## Development Workflow: Spec → Code

THESE INSTRUCTIONS ARE CRITICAL!

They dramatically improve the quality of the work you create.

### Phase 1: Requirements First

When asked to implement any feature or make changes, ALWAYS start by asking:
"Should I create a Spec for this task first?"

IFF user agrees:

- Create a markdown file in `.claude/scopes/FeatureName.md`
- Interview the user to clarify:
- Purpose & user problem
- Success criteria
- Scope & constraints
- Technical considerations
- Out of scope items

### Phase 2: Review & Refine

After drafting the Spec:

- Present it to the user
- Ask: "Does this capture your intent? Any changes needed?"
- Iterate until user approves
- End with: "Spec looks good? Type 'GO!' when ready to implement"

### Phase 3: Implementation

ONLY after user types "GO!" or explicitly approves:

- Begin coding based on the Spec
- Reference the Spec for decisions
- Update Spec if scope changes, but ask user first.

### File Organization

```

.claude/
├── scopes/
│ ├── FeatureName.md # Shared/committed Specs
│ └── OtherFeature.md # Other Specs, for future or past work

```

**Remember: Think first, ask clarifying questions, _then_ code. The Spec is your north star.**
2 changes: 1 addition & 1 deletion src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export function makeCoderSdk(
restClient.getAxiosInstance().interceptors.response.use(
(r) => r,
async (err) => {
throw await CertificateError.maybeWrap(err, baseUrl, storage);
throw await CertificateError.maybeWrap(err, baseUrl);
},
);

Expand Down
5 changes: 2 additions & 3 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as vscode from "vscode";
import { makeCoderSdk, needToken } from "./api";
import { extractAgents } from "./api-helper";
import { CertificateError } from "./error";
import { logger } from "./logger";
import { Storage } from "./storage";
import { toRemoteAuthority, toSafeHost } from "./util";
import { OpenableTreeItem } from "./workspacesProvider";
Expand Down Expand Up @@ -245,9 +246,7 @@ export class Commands {
} catch (err) {
const message = getErrorMessage(err, "no response from the server");
if (isAutologin) {
this.storage.writeToCoderOutputChannel(
`Failed to log in to Coder server: ${message}`,
);
logger.info(`Failed to log in to Coder server: ${message}`);
} else {
this.vscodeProposed.window.showErrorMessage(
"Failed to log in to Coder server",
Expand Down
27 changes: 15 additions & 12 deletions src/error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,19 @@ const isElectron =
// TODO: Remove the vscode mock once we revert the testing framework.
beforeAll(() => {
vi.mock("vscode", () => {
return {};
return {
workspace: {
getConfiguration: vi.fn().mockReturnValue({
get: vi.fn().mockReturnValue(false),
}),
onDidChangeConfiguration: vi.fn().mockReturnValue({
dispose: vi.fn(),
}),
},
};
});
});

const logger = {
writeToCoderOutputChannel(message: string) {
throw new Error(message);
},
};

const disposers: (() => void)[] = [];
afterAll(() => {
disposers.forEach((d) => d());
Expand Down Expand Up @@ -89,7 +92,7 @@ it("detects partial chains", async () => {
try {
await request;
} catch (error) {
const wrapped = await CertificateError.maybeWrap(error, address, logger);
const wrapped = await CertificateError.maybeWrap(error, address);
expect(wrapped instanceof CertificateError).toBeTruthy();
expect((wrapped as CertificateError).x509Err).toBe(X509_ERR.PARTIAL_CHAIN);
}
Expand Down Expand Up @@ -126,7 +129,7 @@ it("detects self-signed certificates without signing capability", async () => {
try {
await request;
} catch (error) {
const wrapped = await CertificateError.maybeWrap(error, address, logger);
const wrapped = await CertificateError.maybeWrap(error, address);
expect(wrapped instanceof CertificateError).toBeTruthy();
expect((wrapped as CertificateError).x509Err).toBe(X509_ERR.NON_SIGNING);
}
Expand Down Expand Up @@ -157,7 +160,7 @@ it("detects self-signed certificates", async () => {
try {
await request;
} catch (error) {
const wrapped = await CertificateError.maybeWrap(error, address, logger);
const wrapped = await CertificateError.maybeWrap(error, address);
expect(wrapped instanceof CertificateError).toBeTruthy();
expect((wrapped as CertificateError).x509Err).toBe(X509_ERR.UNTRUSTED_LEAF);
}
Expand Down Expand Up @@ -200,7 +203,7 @@ it("detects an untrusted chain", async () => {
try {
await request;
} catch (error) {
const wrapped = await CertificateError.maybeWrap(error, address, logger);
const wrapped = await CertificateError.maybeWrap(error, address);
expect(wrapped instanceof CertificateError).toBeTruthy();
expect((wrapped as CertificateError).x509Err).toBe(
X509_ERR.UNTRUSTED_CHAIN,
Expand Down Expand Up @@ -247,7 +250,7 @@ it("falls back with different error", async () => {
try {
await request;
} catch (error) {
const wrapped = await CertificateError.maybeWrap(error, "1", logger);
const wrapped = await CertificateError.maybeWrap(error, "1");
expect(wrapped instanceof CertificateError).toBeFalsy();
expect((wrapped as Error).message).toMatch(/failed with status code 500/);
}
Expand Down
8 changes: 2 additions & 6 deletions src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { isApiError, isApiErrorResponse } from "coder/site/src/api/errors";
import * as forge from "node-forge";
import * as tls from "tls";
import * as vscode from "vscode";
import { logger } from "./logger";

// X509_ERR_CODE represents error codes as returned from BoringSSL/OpenSSL.
export enum X509_ERR_CODE {
Expand All @@ -21,10 +22,6 @@ export enum X509_ERR {
UNTRUSTED_CHAIN = "Your Coder deployment's certificate chain does not appear to be trusted by this system. The root of the certificate chain must be added to this system's trust store. ",
}

export interface Logger {
writeToCoderOutputChannel(message: string): void;
}

interface KeyUsage {
keyCertSign: boolean;
}
Expand All @@ -47,7 +44,6 @@ export class CertificateError extends Error {
static async maybeWrap<T>(
err: T,
address: string,
logger: Logger,
): Promise<CertificateError | T> {
if (isAxiosError(err)) {
switch (err.code) {
Expand All @@ -59,7 +55,7 @@ export class CertificateError extends Error {
await CertificateError.determineVerifyErrorCause(address);
return new CertificateError(err.message, cause);
} catch (error) {
logger.writeToCoderOutputChannel(
logger.info(
`Failed to parse certificate from ${address}: ${error}`,
);
break;
Expand Down
27 changes: 13 additions & 14 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { makeCoderSdk, needToken } from "./api";
import { errToStr } from "./api-helper";
import { Commands } from "./commands";
import { CertificateError, getErrorDetail } from "./error";
import { logger } from "./logger";
import { Remote } from "./remote";
import { Storage } from "./storage";
import { toSafeHost } from "./util";
Expand Down Expand Up @@ -48,6 +49,10 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
}

const output = vscode.window.createOutputChannel("Coder");

// Initialize logger with the output channel
logger.initialize(output);

const storage = new Storage(
output,
ctx.globalState,
Expand Down Expand Up @@ -317,7 +322,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
}
} catch (ex) {
if (ex instanceof CertificateError) {
storage.writeToCoderOutputChannel(ex.x509Err || ex.message);
logger.info(ex.x509Err || ex.message);
await ex.showModal("Failed to open workspace");
} else if (isAxiosError(ex)) {
const msg = getErrorMessage(ex, "None");
Expand All @@ -326,7 +331,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
const method = ex.config?.method?.toUpperCase() || "request";
const status = ex.response?.status || "None";
const message = `API ${method} to '${urlString}' failed.\nStatus code: ${status}\nMessage: ${msg}\nDetail: ${detail}`;
storage.writeToCoderOutputChannel(message);
logger.info(message);
await vscodeProposed.window.showErrorMessage(
"Failed to open workspace",
{
Expand All @@ -337,7 +342,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
);
} else {
const message = errToStr(ex, "No error message was provided");
storage.writeToCoderOutputChannel(message);
logger.info(message);
await vscodeProposed.window.showErrorMessage(
"Failed to open workspace",
{
Expand All @@ -356,14 +361,12 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
// See if the plugin client is authenticated.
const baseUrl = restClient.getAxiosInstance().defaults.baseURL;
if (baseUrl) {
storage.writeToCoderOutputChannel(
`Logged in to ${baseUrl}; checking credentials`,
);
logger.info(`Logged in to ${baseUrl}; checking credentials`);
restClient
.getAuthenticatedUser()
.then(async (user) => {
if (user && user.roles) {
storage.writeToCoderOutputChannel("Credentials are valid");
logger.info("Credentials are valid");
vscode.commands.executeCommand(
"setContext",
"coder.authenticated",
Expand All @@ -381,17 +384,13 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
myWorkspacesProvider.fetchAndRefresh();
allWorkspacesProvider.fetchAndRefresh();
} else {
storage.writeToCoderOutputChannel(
`No error, but got unexpected response: ${user}`,
);
logger.info(`No error, but got unexpected response: ${user}`);
}
})
.catch((error) => {
// This should be a failure to make the request, like the header command
// errored.
storage.writeToCoderOutputChannel(
`Failed to check user authentication: ${error.message}`,
);
logger.info(`Failed to check user authentication: ${error.message}`);
vscode.window.showErrorMessage(
`Failed to check user authentication: ${error.message}`,
);
Expand All @@ -400,7 +399,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
vscode.commands.executeCommand("setContext", "coder.loaded", true);
});
} else {
storage.writeToCoderOutputChannel("Not currently logged in");
logger.info("Not currently logged in");
vscode.commands.executeCommand("setContext", "coder.loaded", true);

// Handle autologin, if not already logged in.
Expand Down
Loading