Skip to content

Conversation

afprtclr
Copy link
Contributor

@afprtclr afprtclr commented Aug 13, 2025

@afprtclr afprtclr force-pushed the feature/2511-showing-empty-messages branch from f95b9c0 to 293120f Compare August 13, 2025 12:31
@afprtclr afprtclr requested a review from danielmarbach August 13, 2025 14:45
@afprtclr afprtclr marked this pull request as ready for review August 13, 2025 14:45
<div v-if="bodyState.not_found" class="alert alert-info">Could not find the message body. This could be because the message URL is invalid or the corresponding message was processed and is no longer tracked by ServiceControl.</div>
<div v-else-if="bodyState.failed_to_load" class="alert alert-info">Message body unavailable.</div>
<LoadingSpinner v-else-if="bodyState.loading" />
<div v-else-if="body === ''" class="alert alert-info">Body was too large and not stored. Edit ServiceControl/MaxBodySizeToStore to be larger in the ServiceControl configuration.</div>
Copy link
Contributor

@soujay soujay Aug 13, 2025

Choose a reason for hiding this comment

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

Suggestion - it will be good to link to the doc as well
https://docs.particular.net/servicecontrol/audit-instances/configuration#performance-tuning-servicecontrol-auditmaxbodysizetostore

Also not a great fan of checking for empty . We could tie to a new empty state in 'bodystate'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed both. Please let me know if that looks better.

@danielmarbach
Copy link
Contributor

@afprtclr I ran out of time today to properly test things, but my recommendation would be something like #2586

We probably also need to implement the same for the SagaDiagram in the SagaDiagramStore.

@afprtclr afprtclr force-pushed the feature/2511-showing-empty-messages branch 3 times, most recently from a4d2220 to 5fb8f03 Compare August 14, 2025 09:22
@afprtclr afprtclr force-pushed the feature/2511-showing-empty-messages branch from 5fb8f03 to 44ac18a Compare August 15, 2025 06:47
@afprtclr
Copy link
Contributor Author

@danielmarbach Thank you. Your approach is much better. I took it into my branch.

Regarding sagas: I can't reproduce the behavior of "empty body" there. ServicePulse happily showed the saga state with 10MB data. When I increased it to 100MB, the audit instance broke and couldn't retrieve the message from RabbitMQ. I'm not sure it would drop the too big saga state. Are you sure it happens (and where)?

@danielmarbach
Copy link
Contributor

@afprtclr that's interesting. I pushed a commit showing the path that reaches to download the message body from the same endpoint URL, if I'm not mistaken, and therefore should also deal with the 204 returned. I haven't had a chance to wrap my head around the specs but maybe we can also add one or two specs to both code paths

@afprtclr
Copy link
Contributor Author

@danielmarbach I can merge your proposal, but I'm not sure we should add it if 204 is never returned for saga (which would render the code redundant).

@afprtclr afprtclr merged commit f3e6572 into master Aug 18, 2025
5 checks passed
@afprtclr afprtclr deleted the feature/2511-showing-empty-messages branch August 18, 2025 12:57
@afprtclr afprtclr added the Type: Bug Type: Bug label Aug 20, 2025
@afprtclr afprtclr added this to the 2.2.0 milestone Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants