Skip to content

Use 204 to express no_content #2586

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

Closed
wants to merge 2 commits into from
Closed

Use 204 to express no_content #2586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
293120f
Indicating when the body wasn't stored for the audited message
afprtclr Aug 13, 2025
6ecbaa9
Use 204 to express no_content
danielmarbach Aug 13, 2025
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
1 change: 1 addition & 0 deletions src/Frontend/src/components/messages/BodyView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const body = computed(() => bodyState.value.data.value);
<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="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 •
edited
Loading

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).

<CodeEditor v-else-if="body !== undefined && contentType.isSupported" :model-value="body" :language="contentType.language" :read-only="true" :show-gutter="true"></CodeEditor>
<div v-else-if="body && !contentType.isSupported" class="alert alert-warning">Message body cannot be displayed because content type "{{ bodyState.data.content_type }}" is not supported.</div>
</div>
Expand Down
10 changes: 8 additions & 2 deletions src/Frontend/src/stores/MessageStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ interface Model {

export const useMessageStore = defineStore("MessageStore", () => {
const headers = ref<DataContainer<Header[]>>({ data: [] });
const body = ref<DataContainer<{ value?: string; content_type?: string }>>({ data: {} });
const body = ref<DataContainer<{ value?: string; content_type?: string; no_content?: boolean }>>({ data: {} });
const state = reactive<DataContainer<Model>>({ data: { failure_metadata: {}, failure_status: {}, dialog_status: {}, invoked_saga: {} } });
let bodyLoadedId = "";
let conversationLoadedId = "";
Expand Down Expand Up @@ -204,6 +204,12 @@ export const useMessageStore = defineStore("MessageStore", () => {
return;
}

if (response.status === 204) {
body.value.data.no_content = true;

return;
}

const contentType = response.headers.get("content-type");
body.value.data.content_type = contentType ?? "text/plain";
body.value.data.value = await response.text();
Expand Down Expand Up @@ -288,7 +294,7 @@ export const useMessageStore = defineStore("MessageStore", () => {

await downloadBody();

if (!(body.value.not_found || body.value.failed_to_load)) {
if (!(body.value.not_found || body.value.failed_to_load || body.value.data.no_content)) {
exportString += "\n\nMESSAGE BODY\n";
exportString += body.value.data.value;
}
Expand Down
Loading