Skip to content

Conversation

@bhufmann
Copy link
Contributor

@bhufmann bhufmann commented Jul 10, 2025

What it does

When an error is created when processing a client request, return an ErrorResponse JSON object that includes

  • title: short, human-readable description of the error
  • detail: optional, human-readable explanation of the error

This standardizes the error response to common data structure. The field names and purpose are inspired by RFC 9457.

For 409 errors (trace or experiment) return extended ErrorResponse containing the conflicting trace or experiment respectively.

This commit also updates the swagger documentation in respect to the ErrorResponses accordingly.

Contributes to fix issue:

eclipse-cdt-cloud/trace-server-protocol#122

How to test

  • Verify using one example use case and see the returned data structure
  • Thorough review to make sure that no case was missed.

Follow-ups

Update TSP specification.

Review checklist

  • As an author, I have thoroughly tested my changes and carefully followed the instructions in this template

@bhufmann bhufmann force-pushed the errorResponse branch 4 times, most recently from 15cd16f to 8f746a7 Compare July 11, 2025 14:51
@bhufmann bhufmann changed the title server: Introduce ErrorResponse and use it everywhere server: Introduce ErrorResponse and use it in error responses Jul 11, 2025
@bhufmann bhufmann requested a review from MatthewKhouzam July 11, 2025 19:42
@marcdumais-work
Copy link
Contributor

While reviewing eclipse-cdt-cloud/trace-server-protocol#126, a couple of "409" (conflict) responses came under scrutiny. Though they are error cases, the current ErrorResponse schema is not adequate to return all pertinent information, for these more complex cases.

The conclusion:

To cover such cases, @bhufmann suggests:

add an optional detail field in the ErrorResponse, which will [contain] a free-from map and in case of 409 I'll add such [contextually useful] information [in the new field]

MatthewKhouzam
MatthewKhouzam previously approved these changes Jul 17, 2025
@bhufmann
Copy link
Contributor Author

While reviewing eclipse-cdt-cloud/trace-server-protocol#126, a couple of "409" (conflict) responses came under scrutiny. Though they are error cases, the current ErrorResponse schema is not adequate to return all pertinent information, for these more complex cases.

The conclusion:

To cover such cases, @bhufmann suggests:

add an optional detail field in the ErrorResponse, which will [contain] a free-from map and in case of 409 I'll add such [contextually useful] information [in the new field]

Yes, I'll add an option detail field to the ErrorResponse, that can be filled accordingly.

@bhufmann bhufmann force-pushed the errorResponse branch 4 times, most recently from fccf3e5 to 16b6884 Compare July 18, 2025 21:08
When an error is created when processing a client request, return
an ErrorResponse JSON object that includes
- title: short, human-readable description of the error
- detail: optional, human-readable explanation of the error

This standardizes the error response to common data structure. The
field names and purpose are inspired by RFC 9457.

For 409 errors (trace or experiment) return extended ErrorResponse
containing the conflicting trace or experiment respectively.

This commit also updates the swagger documentation in respect to the
ErrorResponses accordingly.

This commit also fixes some incorrect or missing the swagger
definitions.

Add description to Trace and Experiment model so that they are not
overridden by the experiment/trace error description.

Contributes to fix issue:

eclipse-cdt-cloud/trace-server-protocol#122

Signed-off-by: Bernd Hufmann <[email protected]>
Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

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

LGTM. I am approving but I am not sure it changes everywhere.

@bhufmann bhufmann merged commit cae954f into eclipse-tracecompass-incubator:master Jul 22, 2025
2 checks passed
@bhufmann bhufmann deleted the errorResponse branch July 22, 2025 20:40
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