Skip to content

fix(api): resolve infinite loading on trace/service fetch errors #3079 #3590

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

Open
Champbreed wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(api): resolve infinite loading on trace/service fetch errors #3079 #3590

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3be9f40
fix(api): prevent infinite loading by surfacing backend errors
Champbreed Mar 11, 2026
14480b9
feat: add error handling guards for Jaeger v3 API
Champbreed Mar 11, 2026
8dbc5b3
Update packages/jaeger-ui/src/api/v3/client.ts
Champbreed Mar 11, 2026
b279ad7
Merge branch 'main' into fix/infinite-loader
Champbreed Mar 11, 2026
58a2a6d
Update packages/jaeger-ui/src/api/v3/client.ts
Champbreed Mar 11, 2026
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
73 changes: 54 additions & 19 deletions packages/jaeger-ui/src/api/v3/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,37 @@ import { ServicesResponseSchema, OperationsResponseSchema } from './schemas';
export class JaegerClient {
private apiRoot = prefixUrl('/api/v3');

/**
* Helper to handle Jaeger v3 specific error arrays in 200 OK responses.
* Throws an Error when the response body contains a non-empty `errors` array,
* surfacing the raw concatenated error messages so public methods can wrap them with context.
*/
private throwIfBodyHasErrors(data: any): void {
if (data?.errors && Array.isArray(data.errors) && data.errors.length > 0) {
const messages = data.errors.map((e: { msg: string }) => e.msg).join(', ');
throw new Error(messages);
}
}

/**
* Fetch the list of services from the Jaeger API.
* @returns Promise<string[]> - Array of service names
*/
async fetchServices(): Promise<string[]> {
const response = await this.fetchWithTimeout(`${this.apiRoot}/services`);
if (!response.ok) {
throw new Error(`Failed to fetch services: ${response.status} ${response.statusText}`);
}
const data = await response.json();
const context = 'Failed to fetch services';
try {
const response = await this.fetchWithTimeout(`${this.apiRoot}/services`);
const data = await response.json();

this.throwIfBodyHasErrors(data);

Comment on lines 33 to 40
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

fetchWithTimeout() now throws on non-2xx responses, but fetchServices() no longer wraps those errors. As a result, callers/UI will likely show a generic message like "404 Not Found" instead of a contextual "Failed to fetch services: ...". Consider catching errors from fetchWithTimeout() here and rethrowing with endpoint context (preserving the original error message as the cause).

Copilot uses AI. Check for mistakes.
// Runtime validation with Zod
const validated = ServicesResponseSchema.parse(data);
return validated.services;
const validated = ServicesResponseSchema.parse(data);
Comment on lines +37 to +41
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

New behavior: fetchServices()/fetchSpanNames() now throw when the JSON body contains an errors array even if the HTTP status is 200. There are currently no v3 client unit tests covering this path; please add tests for (1) 200 OK with errors, and (2) non-OK responses with a JSON {errors:[...]} body to ensure the infinite-loading regression stays fixed.

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +41
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

New behavior: throwIfBodyHasErrors turns 200 OK responses with an errors array into failures. There are existing unit tests for this client, but none cover the 200-with-errors case. Add tests for both fetchServices and fetchSpanNames where ok: true and the JSON body contains a non-empty errors array, asserting the promise rejects and the message is surfaced.

Copilot uses AI. Check for mistakes.
return validated.services;
} catch (error) {
// Copilot Fix: Preserve original error as cause and avoid undefined messages
const message = error instanceof Error ? error.message : String(error);
throw new Error(`${context}: ${message}`);
}
Comment on lines +43 to +47
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The catch block wraps any failure with new Error(...), which drops the original stack trace and can also produce ...: undefined if a non-Error is thrown/rejected. Consider preserving the original error as cause and extracting the message via error instanceof Error ? error.message : String(error) (and avoid re-wrapping when the error is already contextualized).

Copilot uses AI. Check for mistakes.
}

/**
Expand All @@ -36,19 +53,21 @@ export class JaegerClient {
* @returns Promise<{ name: string; spanKind: string }[]> - Array of span name objects
*/
async fetchSpanNames(service: string): Promise<{ name: string; spanKind: string }[]> {
const response = await this.fetchWithTimeout(
`${this.apiRoot}/operations?service=${encodeURIComponent(service)}`
);
if (!response.ok) {
throw new Error(
`Failed to fetch span names for service "${service}": ${response.status} ${response.statusText}`
const context = `Failed to fetch span names for service "${service}"`;
try {
const response = await this.fetchWithTimeout(
`${this.apiRoot}/operations?service=${encodeURIComponent(service)}`
);
}
const data = await response.json();
const data = await response.json();

this.throwIfBodyHasErrors(data);

Comment on lines 17 to 64
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

New behavior is introduced to throw on data.errors even for 200 OK responses, and to extract error messages from errors arrays in non-OK responses. There are existing unit tests for this client, but none that assert these new paths (e.g., ok: true with { errors: [...] }, or ok: false with { errors: [...] }) including the resulting error message. Adding tests would help prevent regressions of the infinite-loading fix.

Copilot uses AI. Check for mistakes.
// Runtime validation with Zod
const validated = OperationsResponseSchema.parse(data);
return validated.operations;
const validated = OperationsResponseSchema.parse(data);
return validated.operations;
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
throw new Error(`${context}: ${message}`);
}
Comment on lines +67 to +70
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Same as fetchServices(): wrapping with new Error(...) loses the original stack/cause and (error as Error).message can be undefined for non-Error throwables. Prefer unknown + instanceof Error narrowing and preserve cause when adding context.

Copilot uses AI. Check for mistakes.
Comment on lines +67 to +70
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Same as fetchServices: this catch block wraps errors but drops the original error object/stack. Prefer attaching the original error via cause (or otherwise preserving it) so callers/debugging can inspect the underlying failure (HTTP error vs validation error vs timeout).

Copilot uses AI. Check for mistakes.
}

/**
Expand All @@ -64,6 +83,22 @@ export class JaegerClient {

try {
const response = await fetch(url, { signal: controller.signal });

if (!response.ok) {
let errorDetail = response.statusText;
try {
const errorBody: any = await response.json();
if (Array.isArray(errorBody?.errors) && errorBody.errors.length > 0) {
errorDetail = errorBody.errors.map((e: { msg: string }) => e.msg).join(', ');
}
} catch {
/* Fallback to statusText if body isn't JSON */
}

// Copilot Fix: Richer error including URL as requested
throw new Error(`${response.status} (${url}) ${errorDetail}`);
}
Comment on lines +87 to +100
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

This change alters the error handling contract (non-OK responses now throw from fetchWithTimeout, and 200 OK responses with errors now throw). The existing client.test.ts suite has cases asserting the previous error messages/paths and should be updated, and new tests should be added for the "200 with errors array" case for both fetchServices and fetchSpanNames to prevent regressions.

Copilot uses AI. Check for mistakes.
Comment on lines +87 to +100
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

fetchWithTimeout() now parses the response body and throws when !response.ok, but the v3 client tests don't appear to exercise this behavior directly (e.g., ensuring an HTTP 4xx/5xx causes a rejection and that errors in the error body are surfaced). Adding a focused unit test for this branch will help prevent regressions and would have caught message/behavior changes in callers.

Copilot uses AI. Check for mistakes.
Comment on lines +96 to +100
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Throwing only ${response.status} ${errorDetail} here changes the error message contract for callers like fetchServices/fetchSpanNames (they can no longer prepend context like "Failed to fetch services"), and will break the existing unit tests that assert those messages. Consider throwing a richer error (including URL) that callers can wrap, or catch/rethrow in the public methods to preserve contextual messages.

Copilot uses AI. Check for mistakes.

return response;
} catch (error) {
if (error instanceof Error && error.name === 'AbortError') {
Expand Down
Loading