Skip to content

Conversation

danielmarbach
Copy link
Contributor

@danielmarbach danielmarbach commented Aug 13, 2025

<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>
<div v-else-if="bodyState.data.no_content" class="alert alert-info">Body was too large and not stored. Edit ServiceControl/MaxBodySizeToStore to be larger in the ServiceControl configuration.</div>
Copy link
Member

Choose a reason for hiding this comment

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

Is it accurate to say that we only return 204 when the body is not stored because it was too large?
And we return 404 when the body is not found at all?

Copy link
Contributor Author

@danielmarbach danielmarbach Aug 14, 2025

Choose a reason for hiding this comment

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

@johnsimons it used to be the case, at least in the past. I can double-check a few things. The logic was always that when the body is not stored, the BodyNotStored metadata was added which then was the indicator for a record to be there and not null but no content being stored.

https://github.com/Particular/ServiceControl/blob/2d7b6e608b7a5c5203142716b7d136b6d95e4294/src/ServiceControl.Audit.Persistence/BodyStorageEnricher.cs#L27-L31

but it seems now we always set HasResult to true in Raven

https://github.com/Particular/ServiceControl/blob/2d7b6e608b7a5c5203142716b7d136b6d95e4294/src/ServiceControl.Persistence.RavenDB/RavenAttachmentsBodyStorage.cs#L60

but the InMemory one has the correct previous behavior

https://github.com/Particular/ServiceControl/blob/2d7b6e608b7a5c5203142716b7d136b6d95e4294/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAuditDataStore.cs#L168

Have to do some more research directly in the code. Like I said I ran out of time yesterday ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is indeed like I said. MessageBodyView differs between NotFound and NoContent which is respected when no attachement could be found and no attachements are stored when it is over the configured max size (else branch).

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

Base branch has been changed to follow this approach
Closing

@danielmarbach danielmarbach deleted the feature/2511-showing-empty-messages-daniel branch August 16, 2025 09:28
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