Skip to content

fix: add error messages to provider status for misconfig#407

Merged
reecebedding merged 2 commits into
mainfrom
fix/notification-misconfig
May 27, 2026
Merged

fix: add error messages to provider status for misconfig#407
reecebedding merged 2 commits into
mainfrom
fix/notification-misconfig

Conversation

@reecebedding
Copy link
Copy Markdown
Member

@reecebedding reecebedding commented May 27, 2026

Summary by CodeRabbit

  • New Features
    • Notification providers now surface per-provider error messages in the provider list so users can see configuration failures (e.g., auth errors).
  • Documentation
    • API schema and docs updated to include the new provider-level error field.
  • Tests
    • Added tests to verify provider error reporting and resolver retry behavior.

Review Change Stack

Copilot AI review requested due to automatic review settings May 27, 2026 14:49
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fc0f2fd2-1146-4f29-be00-3e57263de52e

📥 Commits

Reviewing files that changed from the base of the PR and between 633e6cf and 34de3d7.

📒 Files selected for processing (3)
  • docs/docs.go
  • docs/swagger.json
  • docs/swagger.yaml

📝 Walkthrough

Walkthrough

Adds an Error string to ProviderMetadata, surfaces provider workspace-resolution errors in the ListNotificationProviders response, updates the Slack provider to cache and expose resolver errors, extends tests, and updates OpenAPI/docs to include the new error field.

Changes

Provider Error Exposure

Layer / File(s) Summary
ProviderMetadata error field
internal/service/notification/transport.go
Adds Error string to ProviderMetadata for provider-level errors.
Slack provider error tracking
internal/service/notification/providers/slack/provider.go, internal/service/notification/providers/slack/provider_test.go
Adds workspaceConfigurationErr to Provider, changes workspaceConfigurationDetails to return (config, error), caches errors and configuration, and sets metadata.Error on resolution failure; tests assert error propagation and recovery.
Handler error response and tests
internal/api/handler/notifications.go, internal/api/handler/notifications_test.go
availableNotificationProviderResponse gains error field; ListNotificationProviders populates it from provider metadata. Tests add stubProviderCatalog and TestListNotificationProvidersReturnsProviderErrors to verify API payload includes errors.
API docs and schema
docs/docs.go, docs/swagger.json, docs/swagger.yaml
Documentation and OpenAPI schema updated to include the new error property on the availableNotificationProviderResponse schema.
sequenceDiagram
  participant Client as API Client
  participant Handler as NotificationsHandler
  participant Provider as SlackProvider
  participant Resolver as WorkspaceResolver
  participant Docs as OpenAPI

  Client->>Handler: GET /notifications/providers
  Handler->>Provider: ProviderMetadata()
  Provider->>Provider: workspaceConfigurationDetails()
  Provider->>Resolver: ResolveWorkspaceConfiguration()
  alt Resolver fails
    Resolver-->>Provider: error
    Provider->>Provider: cache workspaceConfigurationErr
    Provider-->>Handler: ProviderMetadata{Error: "..."}
  else Resolver succeeds
    Resolver-->>Provider: configuration
    Provider->>Provider: cache configuration
    Provider-->>Handler: ProviderMetadata{Metadata: {...}, Error: ""}
  end
  Handler-->>Client: JSON list including `error` field
  Handler->>Docs: schema includes `error` (docs updated)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I dug through code where errors hide,
I found the slack where secrets bide.
Now metadata speaks its rueful part,
The handler shows the provider's heart.
Hooray — small changes, clearer art!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: add error messages to provider status for misconfig' accurately describes the main change: adding error fields to notification provider responses to expose misconfiguration issues.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds provider-level error details to notification provider metadata so the admin “list providers” endpoint can surface misconfiguration/connection issues (e.g., Slack auth failures) alongside enabled status and other metadata.

Changes:

  • Extend notification.ProviderMetadata to include an Error string.
  • Populate Slack provider metadata with resolver errors when workspace configuration lookup fails.
  • Expose provider errors via /admin/notifications/providers response and add tests for both Slack provider metadata and the handler response.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/service/notification/transport.go Adds Error field to the shared ProviderMetadata struct.
internal/service/notification/providers/slack/provider.go Returns workspace configuration lookup errors and surfaces them in Slack provider metadata.
internal/service/notification/providers/slack/provider_test.go Verifies Slack provider metadata includes/clears error across retry and recovery.
internal/api/handler/notifications.go Adds error field to admin provider list response payload.
internal/api/handler/notifications_test.go Tests that the admin provider list endpoint returns provider errors in JSON.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

workspaceConfigurationMu sync.Mutex
workspaceConfigurationLoaded bool
workspaceConfiguration slacksvc.WorkspaceConfiguration
workspaceConfigurationErr error
Copy link
Copy Markdown
Contributor

@gusfcarvalho gusfcarvalho left a comment

Choose a reason for hiding this comment

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

pending swagger gen ;)

@reecebedding reecebedding merged commit 8e1d435 into main May 27, 2026
5 checks passed
@reecebedding reecebedding deleted the fix/notification-misconfig branch May 27, 2026 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants