Skip to content

[TT-15954] Make org session fetch non-blocking #7480

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.

Sign up for GitHub

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
imogenkraak wants to merge 21 commits into
base: master
Choose a base branch
from

Make storage layer context aware

bc5f13a
Select commit
Loading
Failed to load commit list.
Draft

[TT-15954] Make org session fetch non-blocking #7480

Make storage layer context aware
bc5f13a
Select commit
Loading
Failed to load commit list.
probelabs / Visor: quality failed Nov 6, 2025 in 5m 27s

🚨 Check Failed

quality check failed because fail_if condition was met.

Details

📊 Summary

  • Total Issues: 3
  • Critical Issues: 1
  • Error Issues: 1
  • Warning Issues: 1

🔍 Failure Condition Results

❌ Failed Conditions

  • global_fail_if: Global failure condition met
    • ⚠️ Severity: Error

🐛 Issues by Category

🏗️ Architecture (1)

  • 🚨 storage/mdcb_storage.go:293 - The context-aware methods added to the MdcbStorage, RPCStorageHandler, and LDAPStorageHandler do not use the provided context. They delegate to the old, blocking methods, which will cause goroutines to leak if the backend is unresponsive, defeating the purpose of making the session fetch non-blocking. This can lead to resource exhaustion and gateway failure.

🧠 Logic (2)

  • ⚠️ gateway/middleware_test.go:628 - The test case "should handle panic during fetch gracefully" does not actually induce a panic. It only verifies that the test runner does not crash, which fails to confirm that the recover() block in fetchOrgSessionWithTimeout is effective. The test provides a false sense of security about the panic handling mechanism.
  • ❌ system:0 - Global failure condition met

Powered by Visor from Probelabs

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

Annotations

Check failure on line 295 in storage/mdcb_storage.go

See this annotation in the file changed.

@probelabs probelabs / Visor: quality

architecture Issue 

The context-aware methods added to the `MdcbStorage`, `RPCStorageHandler`, and `LDAPStorageHandler` do not use the provided context. They delegate to the old, blocking methods, which will cause goroutines to leak if the backend is unresponsive, defeating the purpose of making the session fetch non-blocking. This can lead to resource exhaustion and gateway failure.
Raw output 
Modify the `GetKeyContext`, `GetRawKeyContext`, and `GetMultiKeyContext` methods in `storage/mdcb_storage.go`, `gateway/rpc_storage_handler.go`, and `gateway/ldap_auth_handler.go` to use the provided `context.Context`. For RPC and MDCB handlers, this typically involves creating an `http.Request` with `http.NewRequestWithContext` to ensure network operations are cancelled when the context times out.

Check warning on line 636 in gateway/middleware_test.go

See this annotation in the file changed.

@probelabs probelabs / Visor: quality

logic Issue 

The test case "should handle panic during fetch gracefully" does not actually induce a panic. It only verifies that the test runner does not crash, which fails to confirm that the `recover()` block in `fetchOrgSessionWithTimeout` is effective. The test provides a false sense of security about the panic handling mechanism.
Raw output 
Update the `mockStore` to be configurable to panic within the `SessionDetailContext` method. Modify the test to enable this panic behavior and assert that `fetchOrgSessionWithTimeout` returns `found: false` as expected, confirming that the panic was successfully recovered.
View more details on probelabs