fix(bug): Handle errors in response body with status code is 200 #3161

Melonps · 2025-11-21T15:49:05Z

Which problem is this PR solving?

Resolves [Bug]: UI Shows Infinite Loading for Non-Existent Trace ID #3079

Description of the changes

In packages/jaeger-ui/src/api/jaeger.js

Validate response body for errors array even when status is 200
Throw error if errors are present to trigger error handling flow

After this change, I checked to show valid error message in UI, also prevents infinity loading.
Please see the screenshot below:

How was this change tested?

Add 1 test to verify this changing in packages/jaeger-ui/src/api/jaeger.test.js

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
I have run lint and test steps successfully
- for jaeger: make lint test
- for jaeger-ui: npm run lint and npm run test

Signed-off-by: Melonps <[email protected]>

graphite-app · 2025-11-21T15:49:13Z

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

packages/jaeger-ui/src/api/jaeger.test.js

Signed-off-by: Melonps <[email protected]>

yurishkuro · 2025-11-25T00:05:00Z

packages/jaeger-ui/src/api/jaeger.js

    if (response.status < 400) {
-      return response.json();
+      return response.json().then(body => {
+        if (body && Array.isArray(body.errors) && body.errors.length > 0) {


A function called getJSON cannot assume to know the structure of the response. Why do we need to throw exception in this lower level function instead of handling the error where needed?

Thanks for your feedback🙏
I've refactored the code to keep getJSON generic, and moved the Jaeger specific error handling (where 200 responses can contain an errors array) into the searchTraces method itself.

Without this check, these errors would be silently ignored and treated as successful 200 responses.

…etJSON` Signed-off-by: Melonps <[email protected]>

yurishkuro · 2025-11-25T02:53:43Z

packages/jaeger-ui/src/api/jaeger.js

+        error.httpBody = JSON.stringify(body, null, 2);
+        throw error;
+      }
+      return body;


A successful result is not mutually exclusive with some errors being present. This logic is incorrect. The errors need to be checked by the caller.

@yurishkuro
Thanks for the review ! I understand your point now🙇

I see that fetchMultipleTraces already handles this pattern correctly in packages/jaeger-ui/src/reducers/trace.js, where it processes both payload.data and payload.errors separately. I should follow the same approach for searchTraces, right?

I'll need a bit more time to fix it. Will update the PR soon.

Melonps · 2025-11-25T12:28:55Z

@yurishkuro

I misidentified the root cause. The actual problem is a trace ID normalization mismatch:

Frontend sends requests using short trace IDs (e.g., "1", "xyz").
Backend zero-pads them to 32-char: "0000000000000001", "0000000000000xyz", as defined in jaeger-idl
Frontend stores responses under the normalized IDs
Original short IDs remain stuck in FETCH_LOADING state

I think we should normalize trace IDs to 32-char hex strings on the frontend before issuing API requests by 0, since the Jaeger backend zero-pads them and we need the request and response IDs to align.

yurishkuro · 2025-11-25T13:50:14Z

Where does the frontend get the short id from in the first place?

Signed-off-by: Melonps <[email protected]>

graphite-app · 2025-11-25T14:01:15Z

packages/jaeger-ui/src/utils/normalize.ts

+ * normalizeTraceId('1') // '0000000000000001'
+ * normalizeTraceId('xyz') // '0000000000000abc'
+ * normalizeTraceId('abcd1234') // 'abcd1234'


The JSDoc examples are incorrect and misleading:

Line 15: normalizeTraceId('xyz') claims to return '0000000000000abc', but 'xyz' contains no valid hex characters. After cleaning (line 25), it becomes an empty string, triggering line 28-29 to return the original 'xyz' unchanged.

Line 16: normalizeTraceId('abcd1234') claims to return 'abcd1234', but the function pads to 16 characters (line 33), so it should return '00000000abcd1234'.

Fix the documentation:

* @example * normalizeTraceId('1') // '0000000000000001' * normalizeTraceId('xyz') // 'xyz' (invalid, returned unchanged) * normalizeTraceId('abcd1234') // '00000000abcd1234' * normalizeTraceId('1234567890abcdef1234') // '1234567890abcdef1234'

Suggested change

* normalizeTraceId('1') // '0000000000000001'

* normalizeTraceId('xyz') // '0000000000000abc'

* normalizeTraceId('abcd1234') // 'abcd1234'

* normalizeTraceId('1') // '0000000000000001'

* normalizeTraceId('xyz') // 'xyz' (invalid, returned unchanged)

* normalizeTraceId('abcd1234') // '00000000abcd1234'

* normalizeTraceId('1234567890abcdef1234') // '1234567890abcdef1234'

Spotted by Graphite Agent

Is this helpful? React 👍 or 👎 to let us know.

yurishkuro · 2025-11-25T14:05:50Z

packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/TraceIdInput.tsx

-  return <Search addonBefore="Select by Trace ID" enterButton onSearch={selectTrace} />;
+  const handleSearch = React.useCallback(
+    (value: string) => {
+      selectTrace(normalizeTraceId(value));


Are you normalizing raw user input?

@yurishkuro
yes,

Where does the frontend get the short id from in the first place?

Agree that, but I am specifically targeting the scenario where a user manually inputs a Trace ID into the search bar

Sometimes users might input a short ID (e.g., due to a copy paste error or typing only a part of it). Currently, sending this short ID to the backend causes a mismatch with the IDL, leading to an unhandled error that results in the infinite loading state. (UI should show Trace not found.

I agree that we shouldn't handle short IDs internally, but to fix this UX issue (the infinite spinner), I propose applying zero-padding specifically for the search bar input. This ensures the ID format matches what the backend expects (16 or 32 characters) and prevents the UI from hanging.

Actually, It is likely that user input 31-char trace ID in input, because added 0 to tracd Id's head from backend response (ref: # #3079)

if a user inputs invalid ID we should display an error. That the UI keeps spinning in this case is a bug that we need to fix. But we do not need to fix invalid IDs. How do you know their copy&paste omitted the leading zero and not a trailing character?

That makes sense.
I will rework the change to implement client-side validation on the input form. If the user enters a Trace ID with an invalid length, an error will be shown immediately, which will effectively resolve the spinning UI bug while maintaining data integrity.

I'm still learning the ropes, so I really appreciate your feedback. I'll push the fixes shortly.

Melonps · 2025-11-26T00:02:15Z

@yurishkuro

I’m planning to validate the user input as follows.
When the user enters an invalid trace ID, the TraceHeader component will display a validation error in red:

Also considering another approach using packages/jaeger-ui/src/utils/ValidatedFormField.tsx, which would show the validation error in a popover(but, two popovers appearing at the same time)

Signed-off-by: Melonps <[email protected]>

packages/jaeger-ui/src/components/TraceDiff/TraceDiff.tsx

Signed-off-by: Melonps <[email protected]>

Melonps · 2025-11-30T14:41:54Z

@yurishkuro
Hi!, I update this PR, introducing validation function for checking traceId's digit into redux action and pass only valid IDs to fetch.
Along with it, I add ValidateError as type.

yurishkuro · 2025-11-30T17:31:43Z

packages/jaeger-ui/src/utils/trace-id-validation.ts

+
+import { ValidateError } from '../types/validate-error';
+
+export function validateTraceId(traceId: string): ValidateError | null {


The UI has no direct dependency on the trace ID being in any specific format. It's just an opaque string for the UI, it should not be adding any additional semantics to that string.

yurishkuro · 2025-11-30T17:33:36Z

packages/jaeger-ui/src/actions/jaeger-api.js

  ids => ({ ids })
 );

+export const setTraceValidationError = createAction(


why does this need to be at the API level instead of just handled in TraceDiff?

yurishkuro · 2025-11-30T17:34:34Z

packages/jaeger-ui/src/reducers/trace.js

    traces[id] = { error, id, state: fetchedState.ERROR };
  });
  return { ...state, traces };
 }


there is no packages/jaeger-ui/src/reducers/trace.js file in main branch. You fork might be stale.

Melonps added 2 commits November 22, 2025 00:28

fix: handle errors in response body with status code is 200

e81e5da

Signed-off-by: Melonps <[email protected]>

test: add test for errors with HTTP 200 responses in searchTraces()

9394fc0

Signed-off-by: Melonps <[email protected]>

Melonps requested a review from a team as a code owner November 21, 2025 15:49

Melonps requested review from albertteoh and removed request for a team November 21, 2025 15:49

graphite-app bot reviewed Nov 21, 2025

View reviewed changes

packages/jaeger-ui/src/api/jaeger.test.js Outdated Show resolved Hide resolved

fix: fail immediately if the test unexpectedly resolves

0c86609

Signed-off-by: Melonps <[email protected]>

Melonps changed the title ~~fix: handle errors in response body with status code is 200~~ Handle errors in response body with status code is 200 Nov 22, 2025

Melonps changed the title ~~Handle errors in response body with status code is 200~~ fix(bug): Handle errors in response body with status code is 200 Nov 24, 2025

Merge branch 'main' into fix/handle-200-errors

84b9689

yurishkuro reviewed Nov 25, 2025

View reviewed changes

fix: refactored the code to move jaeger-specific error handle from `g…

086ac27

…etJSON` Signed-off-by: Melonps <[email protected]>

yurishkuro requested changes Nov 25, 2025

View reviewed changes

feat: Normalize trace Ids to 16-char with zero-padded in TraceIdInput

d142a15

Signed-off-by: Melonps <[email protected]>

graphite-app bot reviewed Nov 25, 2025

View reviewed changes

yurishkuro reviewed Nov 25, 2025

View reviewed changes

Melonps added 4 commits November 30, 2025 23:04

feat: add redux actions to express validation error

3a43ff9

Signed-off-by: Melonps <[email protected]>

delete: unused changes

af6423e

Signed-off-by: Melonps <[email protected]>

feat: show validation error on UI

7da003c

Signed-off-by: Melonps <[email protected]>

add: setTraceValidationError

41d3be6

Signed-off-by: Melonps <[email protected]>

graphite-app bot reviewed Nov 30, 2025

View reviewed changes

packages/jaeger-ui/src/components/TraceDiff/TraceDiff.tsx Show resolved Hide resolved

fix: Duplicate CSS import

98cd0f5

Signed-off-by: Melonps <[email protected]>

yurishkuro reviewed Nov 30, 2025

View reviewed changes


		import { ValidateError } from '../types/validate-error';

		export function validateTraceId(traceId: string): ValidateError \| null {

fix(bug): Handle errors in response body with status code is 200 #3161

Are you sure you want to change the base?

fix(bug): Handle errors in response body with status code is 200 #3161

Uh oh!

Conversation

Melonps commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

graphite-app bot commented Nov 21, 2025

How to use the Graphite Merge Queue

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Melonps Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Melonps commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yurishkuro commented Nov 25, 2025

Uh oh!

graphite-app bot Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Melonps Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Melonps Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Melonps commented Nov 26, 2025

Uh oh!

Uh oh!

Melonps commented Nov 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Melonps commented Nov 21, 2025 •

edited

Loading

Melonps Nov 25, 2025 •

edited

Loading

Melonps commented Nov 25, 2025 •

edited

Loading

Melonps Nov 25, 2025 •

edited

Loading

Melonps Nov 25, 2025 •

edited

Loading