Skip to content

fix: address security and performance review findings

503a68d
Select commit
Loading
Failed to load commit list.
Draft

feat: add outbound webhooks with at-least-once delivery #325

fix: address security and performance review findings
503a68d
Select commit
Loading
Failed to load commit list.
probelabs / Visor: performance failed Feb 28, 2026 in 1m 16s

🚨 Check Failed

performance check failed because fail_if condition was met.

Details

📊 Summary

  • Total Issues: 4
  • Error Issues: 2
  • Warning Issues: 2

🔍 Failure Condition Results

Failed Conditions

  • global_fail_if: output.issues && output.issues.some(i => i.severity === 'critical' || i.severity === 'error')
    • Severity: ❌ error

Issues by Category

Performance (3)

  • services/webhook_service.go:327 - A new http.Client is created for every webhook delivery in processEvent. This is inefficient as it prevents TCP connection reuse, which can lead to performance degradation and resource exhaustion (e.g., socket descriptors in a TIME_WAIT state) under high load. The http.Client is designed to be reused.
  • ⚠️ api/webhook_handlers.go:152 - The GET /api/v1/webhooks endpoint, implemented by the List handler, retrieves all webhook subscriptions without pagination. If a large number of subscriptions are created, this can lead to excessive memory consumption and slow API responses.
  • ⚠️ services/webhook_service.go:557 - The findMatchingSubscriptions function uses Preload("Topics"), which executes an additional database query to fetch all topics for the matching subscriptions. However, the calling function, HandleEvent, only uses the SubscriptionID and does not use the preloaded Topics data. This results in an unnecessary database query for every event that triggers a webhook.

Logic (1)

  • system:0 - Global failure condition met: output.issues && output.issues.some(i => i.severity === 'critical' || i.severity === 'error')

Powered by Visor from Probelabs

💡 TIP: You can chat with Visor using /visor ask <your question>

Annotations

Check failure on line 327 in services/webhook_service.go

See this annotation in the file changed.

@probelabs probelabs / Visor: performance

performance Issue

A new `http.Client` is created for every webhook delivery in `processEvent`. This is inefficient as it prevents TCP connection reuse, which can lead to performance degradation and resource exhaustion (e.g., socket descriptors in a `TIME_WAIT` state) under high load. The `http.Client` is designed to be reused.
Raw output
Cache and reuse `http.Client` instances. Since transport settings are per-subscription, create a cache that maps a subscription's transport configuration to a shared `http.Client`. This will allow for connection pooling and significantly improve performance for endpoints that receive frequent webhooks.

Check warning on line 152 in api/webhook_handlers.go

See this annotation in the file changed.

@probelabs probelabs / Visor: performance

performance Issue

The `GET /api/v1/webhooks` endpoint, implemented by the `List` handler, retrieves all webhook subscriptions without pagination. If a large number of subscriptions are created, this can lead to excessive memory consumption and slow API responses.
Raw output
Implement pagination for the `List` handler and the underlying `ListWebhooks` service method. Use the existing `getPaginationParams` helper to handle `page` and `page_size` query parameters and apply `LIMIT` and `OFFSET` to the database query, consistent with the `ListDeliveries` handler.

Check warning on line 557 in services/webhook_service.go

See this annotation in the file changed.

@probelabs probelabs / Visor: performance

performance Issue

The `findMatchingSubscriptions` function uses `Preload("Topics")`, which executes an additional database query to fetch all topics for the matching subscriptions. However, the calling function, `HandleEvent`, only uses the `SubscriptionID` and does not use the preloaded `Topics` data. This results in an unnecessary database query for every event that triggers a webhook.
Raw output
Remove the `.Preload("Topics")` call from the database query within the `findMatchingSubscriptions` function to avoid the unnecessary database call.