Skip to content

backend/enh/added-callBecknAPIWithLogging #1227

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

Open
H-C-21 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

backend/enh/added-callBecknAPIWithLogging #1227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/mobility-core/src/Kernel/Utils/Common.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import Kernel.Types.Field as Common
import Kernel.Types.Id (ShortId (ShortId))
import Kernel.Utils.Context as Common
import Kernel.Utils.Error as Common
import Kernel.Utils.Error.BaseError.HTTPError.BecknAPIError (callBecknAPI)
import Kernel.Utils.Error.BaseError.HTTPError.BecknAPIError (callBecknAPI, callBecknAPIWithLogging)
import Kernel.Utils.Error.DB as Common
import Kernel.Utils.Logging as Common
import Kernel.Utils.Servant.Client as Common
Expand Down
31 changes: 31 additions & 0 deletions lib/mobility-core/src/Kernel/Utils/Error/BaseError/HTTPError/BecknAPIError.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ module Kernel.Utils.Error.BaseError.HTTPError.BecknAPIError where
import qualified Data.HashMap.Strict as HM
import EulerHS.Prelude
import qualified EulerHS.Types as ET
import Kernel.Streaming.Kafka.Producer.Types (HasKafkaProducer)
import Kernel.Tools.Metrics.CoreMetrics (CoreMetrics)
import Kernel.Types.Common
import Kernel.Types.Error.BaseError.HTTPError
import Kernel.Utils.InternalAPICallLogging (pushInternalApiCallDataToKafkaWithTextEncodedResp)
import Kernel.Utils.Monitoring.Prometheus.Servant
import Kernel.Utils.Servant.Client
import qualified Kernel.Utils.Text as KUT
import Servant.Client (Client, HasClient)

data BecknAPICallError = BecknAPICallError Text Error
Expand Down Expand Up @@ -65,6 +68,34 @@ callBecknAPI ::
callBecknAPI mbManagerSelector errorCodeMb action api baseUrl internalEndPointHashMap req = do
callBecknAPI' mbManagerSelector errorCodeMb (Just internalEndPointHashMap) baseUrl (ET.client api req) action api

callBecknAPIWithLogging ::
( MonadFlow m,
CoreMetrics m,
IsBecknAPI api req res,
SanitizedUrl api,
HasRequestId r,
MonadReader r m,
HasKafkaProducer r,
ToJSON req
) =>
Maybe ET.ManagerSelector ->
Maybe Text ->
Text ->
Text ->
Maybe Text ->
Proxy api ->
BaseUrl ->
HM.HashMap BaseUrl BaseUrl ->
req ->
m res
callBecknAPIWithLogging mbManagerSelector errorCodeMb action source entityId api baseUrl internalEndPointHashMap req = do
result <- try $ callBecknAPI mbManagerSelector errorCodeMb action api baseUrl internalEndPointHashMap req
fork ("Logging Beckn API call: " <> action) $ do
let resText = either (show :: SomeException -> Text) KUT.encodeToText result
logDebug $ "Beckn API call " <> action <> " result: " <> resText
pushInternalApiCallDataToKafkaWithTextEncodedResp action source entityId (Just req) resText
either throwM return result
Comment on lines +91 to +97
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Kafka payload does not distinguish success from failure.

On Line 94, both a successful res (JSON-encoded via KUT.encodeToText) and a SomeException (rendered via show) are collapsed into a single Text that is pushed to Kafka and written to the debug log. Downstream consumers of the Internal-API-Call-Logs topic have no structured way to tell whether a given record represents a 2xx response or an upstream failure, and exception show output is not valid JSON, so any consumer trying to parse the field as JSON will break on the error path.

Consider tagging the outcome explicitly before encoding, e.g.:

🛠️ Suggested change 
-  fork ("Logging Beckn API call: " <> action) $ do
-    let resText = either (show :: SomeException -> Text) KUT.encodeToText result
-    logDebug $ "Beckn API call " <> action <> " result: " <> resText
-    pushInternalApiCallDataToKafkaWithTextEncodedResp action source entityId (Just req) resText
+  fork ("Logging Beckn API call: " <> action) $ do
+    let resText = case result of
+          Right res -> KUT.encodeToText (Right res :: Either Text res)
+          Left (e :: SomeException) -> KUT.encodeToText (Left (show e) :: Either Text res)
+    logDebug $ "Beckn API call " <> action <> " result: " <> resText
+    pushInternalApiCallDataToKafkaWithTextEncodedResp action source entityId (Just req) resText
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@lib/mobility-core/src/Kernel/Utils/Error/BaseError/HTTPError/BecknAPIError.hs`
around lines 91 - 97, The current callBecknAPIWithLogging collapses success and
exception into a plain Text (resText) which prevents consumers from
distinguishing outcomes and breaks JSON parsers; change it to build a small
structured wrapper (e.g. {status: "success" | "error", payload: <response JSON
or error string>}) and KUT.encodeToText that wrapper before calling
pushInternalApiCallDataToKafkaWithTextEncodedResp and in logDebug, so callers of
pushInternalApiCallDataToKafkaWithTextEncodedResp and readers of
Internal-API-Call-Logs can unambiguously detect success vs failure and safely
parse JSON; use the existing identifiers callBecknAPIWithLogging, result,
pushInternalApiCallDataToKafkaWithTextEncodedResp and KUT.encodeToText to locate
and implement the change.


callBecknAPI' ::
( MonadFlow m,
MonadReader r m,
Expand Down
Loading