Skip to content

Fix: use WARN level and "Notif failure" message for failed notifications (#4800) - #4801

Merged
fgalan merged 5 commits into
masterfrom
fix/improve-notif-error-severity
Jul 24, 2026
Merged

Fix: use WARN level and "Notif failure" message for failed notifications (#4800)#4801
fgalan merged 5 commits into
masterfrom
fix/improve-notif-error-severity

Conversation

@orianar

@orianar orianar commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Issue #4800

@orianar orianar changed the title [WIP] Fix: use WARN level and "Notif failure" message for failed notifications (#4800) Fix: use WARN level and "Notif failure" message for failed notifications (#4800) Jul 23, 2026
@orianar
orianar requested a review from fgalan July 23, 2026 01:01
Comment thread doc/manuals.jp/admin/logs.md Outdated
...

time=2026-03-12T13:16:09.473Z | lvl=INFO | corr=a280a8ae-1e15-11f1-ae2a-080027207a9f; cbnotif=1 | trans=1773321367-657-00000000004 | from=0.0.0.0 | srv=<none> | subsrv=/ | comp=Orion | op=logTracing.cpp[105]:logInfoHttpNotification | msg=Notif delivered (subId: 69b2bc999bb797a5cd07dc74): POST localhost:9997/giveme400, payload (123 bytes): {"subscriptionId":"69b2bc999bb797a5cd07dc74","data":[{"id":"E1","type":"T","A":{"type":"Number","value":1,"metadata":{}}}]}, response code: 400, response payload (93 bytes): {"error": "FakeError", "description": "this is a forged error response for testing purposes"}
time=2026-03-12T13:16:09.473Z | lvl=WARN | corr=a280a8ae-1e15-11f1-ae2a-080027207a9f; cbnotif=1 | trans=1773321367-657-00000000004 | from=0.0.0.0 | srv=<none> | subsrv=/ | comp=Orion | op=logTracing.cpp[105]:logInfoHttpNotification | msg=Notif failure (subId: 69b2bc999bb797a5cd07dc74): POST localhost:9997/giveme400, payload (123 bytes): {"subscriptionId":"69b2bc999bb797a5cd07dc74","data":[{"id":"E1","type":"T","A":{"type":"Number","value":1,"metadata":{}}}]}, response code: 400, response payload (93 bytes): {"error": "FakeError", "description": "this is a forged error response for testing purposes"}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this case should be changed

The "failure" semantics for notifications is "the notification hasn't reached the destination" no matter the response code of the receiver (either 2xx, 4xx or 5xx). This is somehow confirmed by alarm ID 5 type in this table

Image

The change INFO -> WARN and "Notif delivered" -> "Notif failure" is ok for the following cases shown in this documentation:

  • Couldn't connect to server
  • Timeout was reached
  • Couldn't resolve host name

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in #2fff6a3

@orianar
orianar requested a review from fgalan July 23, 2026 14:02
Comment thread src/lib/common/logTracing.cpp Outdated
}

LM_I(("Notif delivered (subId: %s): %s %s%s, payload (%d bytes): %s, response code: %s", subId, verb, endpoint, resource, strlen(payload), effectivePayload, reason));
LM_W(("Notif failure (subId: %s): %s %s%s, payload (%d bytes): %s, response code: %s", subId, verb, endpoint, resource, strlen(payload), effectivePayload, reason));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given that we are changing the structure of the message ("delivered" -> "failure") maybe we should do the same for "response code", as in this case we don't have an actual response code (in the past we did so to have an homogenous format along all the notification log messages)

What about?

Suggested change
LM_W(("Notif failure (subId: %s): %s %s%s, payload (%d bytes): %s, response code: %s", subId, verb, endpoint, resource, strlen(payload), effectivePayload, reason));
LM_W(("Notif failure (subId: %s): %s %s%s, payload (%d bytes): %s, reason: %s", subId, verb, endpoint, resource, strlen(payload), effectivePayload, reason));

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in #ecc22fb

@fgalan fgalan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@fgalan
fgalan merged commit ca9d8a3 into master Jul 24, 2026
17 checks passed
@fgalan
fgalan deleted the fix/improve-notif-error-severity branch July 24, 2026 14:29
@fgalan

fgalan commented Jul 24, 2026

Copy link
Copy Markdown
Member

@fisuda this PR does some modifications in .md files. It would be great if you could have a look and do a PR with the Japanese sync. Thanks in advance!

fisuda added a commit to fisuda/fiware-orion that referenced this pull request Jul 26, 2026
@fisuda

fisuda commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

I sent the PR #4802.
Thanks.

fgalan added a commit that referenced this pull request Jul 27, 2026
(JP) Added documentation about failed notifications (#4801)
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