-
Notifications
You must be signed in to change notification settings - Fork 118
feat: add error handling for extension activation #1741
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughActivation logic in src/dbtPowerUserExtension.ts is wrapped in a try/catch. On activation errors, a telemetry event "extensionActivationError" is sent. Existing initialization steps and the configuration change listener are executed within the try block. No exported/public signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
participant VSCode as VS Code
participant Ext as Extension (dbtPowerUserExtension)
participant Telemetry as Telemetry
VSCode->>Ext: activate()
rect rgb(235, 245, 255)
Ext->>Ext: initialize MCP API, bind context, init walkthrough
Ext->>Ext: detect DBT, init projects, status bars
Ext->>Ext: register onDidChangeConfiguration listener
end
Note over Ext: If any step throws
Ext-->>Telemetry: send("extensionActivationError", details)
Ext-->>VSCode: propagate/complete activation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 48a668d in 55 seconds. Click for details.
- Reviewed
60
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/dbtPowerUserExtension.ts:108
- Draft comment:
The configuration change listener isn’t stored for disposal. Consider adding the returned disposable to context.subscriptions (or your disposables) to avoid potential memory leaks. - Reason this comment was not posted:
Comment was on unchanged code.
2. src/dbtPowerUserExtension.ts:122
- Draft comment:
The catch block handles errors by sending telemetry. Consider typing the caught error (e.g., as 'unknown') if further error introspection is needed. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
Workflow ID: wflow_Y2Sx7WiNXruFutkB
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/dbtPowerUserExtension.ts (4)
108-121
: Dispose the configuration-change listener and narrow the change filter.
- The returned Disposable from onDidChangeConfiguration isn’t stored, so it won’t be cleaned up on dispose. Push it into this.disposables to avoid leaks and duplicate listeners across re-activations.
- Also narrow affectsConfiguration to the exact setting. No need to react to all “dbt” config changes.
Apply:
- workspace.onDidChangeConfiguration((e) => { - if (!e.affectsConfiguration("dbt")) { + const configChangeDisposable = workspace.onDidChangeConfiguration((e) => { + if (!e.affectsConfiguration("dbt.dbtIntegration")) { return; } const newDbtIntegration = workspace .getConfiguration("dbt") .get<string>("dbtIntegration", "core"); if ( dbtIntegration !== newDbtIntegration && ["core", "cloud", "corecommand", "fusion"].includes(newDbtIntegration) ) { commands.executeCommand("workbench.action.reloadWindow"); } - }); + }); + this.disposables.push(configChangeDisposable);
122-124
: Add minimal context to telemetry and log to console for local diagnostics.Enriching the telemetry with a “phase” (or similar) makes triage easier. Logging to console also helps during local dev.
- this.telemetry.sendTelemetryError("extensionActivationError", error); + this.telemetry.sendTelemetryError("extensionActivationError", error, { + phase: "activate", + }); + console.error("DBT Power User: Extension activation failed", error);
104-121
: Consider registering the config listener earlier in activation.If a prior step (e.g., detectDBT or initializeDBTProjects) throws, the listener won’t be registered. Moving the listener registration just after computing dbtIntegration (or to the top of the try block) ensures it’s set even if later steps fail.
97-125
: Add a unit test to assert telemetry on activation failure.A small test that forces one activation step (e.g., updateMcpExtensionApi) to throw and asserts sendTelemetryError("extensionActivationError", …) was called will protect this behavior.
I can draft a Jest test that injects a throwing mcpServer mock and verifies telemetry invocation. Want me to provide it?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/dbtPowerUserExtension.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: AltimateAI/vscode-dbt-power-user#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T17:27:13.149Z
Learning: Applies to src/extension.ts : Wire up new providers in the `DBTPowerUserExtension` class when adding language features.
📚 Learning: 2025-07-30T17:27:13.149Z
Learnt from: CR
PR: AltimateAI/vscode-dbt-power-user#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T17:27:13.149Z
Learning: Applies to src/extension.ts : Wire up new providers in the `DBTPowerUserExtension` class when adding language features.
Applied to files:
src/dbtPowerUserExtension.ts
🧬 Code Graph Analysis (1)
src/dbtPowerUserExtension.ts (2)
src/test/mock/vscode.ts (2)
workspace
(102-119)commands
(77-81)src/dbt_client/dbtTerminal.ts (1)
error
(105-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test (macos-latest)
- GitHub Check: test (windows-latest)
- GitHub Check: test (ubuntu-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
src/dbtPowerUserExtension.ts (1)
97-125
: Good defensive wrapper and telemetry on activation failure.Wrapping activation in try/catch and emitting a dedicated telemetry event is a solid improvement. Behavior remains unchanged on success.
Overview
Problem
Describe the problem you are solving. Mention the ticket/issue if applicable.
Solution
Describe the implemented solution. Add external references if needed.
Screenshot/Demo
A picture is worth a thousand words. Please highlight the changes if applicable.
How to test
Checklist
README.md
updated and added information about my changeImportant
Adds error handling to
activate()
inDBTPowerUserExtension
to send telemetry on activation errors.activate()
method ofDBTPowerUserExtension
to handle errors during extension activation.extensionActivationError
if an error occurs.This description was created by
for 48a668d. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit