Tracing: Add some error handling to JaegerQueryField (#23599)

packages/grafana-data/src/types/appEvents.ts

@@ -6,7 +6,8 @@ export interface AppEvent<T> {
 }
 
 export type AlertPayload = [string, string?];
+export type AlertErrorPayload = [string, (string | Error)?];
 
 export const alertSuccess = eventFactory<AlertPayload>('alert-success');
 export const alertWarning = eventFactory<AlertPayload>('alert-warning');
-export const alertError = eventFactory<AlertPayload>('alert-error');
+export const alertError = eventFactory<AlertErrorPayload>('alert-error');

public/app/core/copy/appNotification.ts

@@ -32,7 +32,11 @@ export const createSuccessNotification = (title: string, text = ''): AppNotifica
   id: Date.now(),
 });
 
-export const createErrorNotification = (title: string, text = '', component?: React.ReactElement): AppNotification => {
+export const createErrorNotification = (
+  title: string,
+  text: string | Error = '',
+  component?: React.ReactElement
+): AppNotification => {
   return {
     ...defaultErrorNotification,
     text: getMessageFromError(text),

public/app/core/utils/errors.test.ts

@@ -15,7 +15,7 @@ describe('errors functions', () => {
 
   describe('when getMessageFromError gets an error object with message field', () => {
     beforeEach(() => {
-      message = getMessageFromError({ message: 'error string' });
+      message = getMessageFromError({ message: 'error string' } as Error);
     });
 
     it('should return the message text', () => {
@@ -25,7 +25,7 @@ describe('errors functions', () => {
 
   describe('when getMessageFromError gets an error object with data.message field', () => {
     beforeEach(() => {
-      message = getMessageFromError({ data: { message: 'error string' } });
+      message = getMessageFromError({ data: { message: 'error string' } } as any);
     });
 
     it('should return the message text', () => {
@@ -35,7 +35,7 @@ describe('errors functions', () => {
 
   describe('when getMessageFromError gets an error object with statusText field', () => {
     beforeEach(() => {
-      message = getMessageFromError({ statusText: 'error string' });
+      message = getMessageFromError({ statusText: 'error string' } as any);
     });
 
     it('should return the statusText text', () => {
@@ -45,7 +45,7 @@ describe('errors functions', () => {
 
   describe('when getMessageFromError gets an error object', () => {
     beforeEach(() => {
-      message = getMessageFromError({ customError: 'error string' });
+      message = getMessageFromError({ customError: 'error string' } as any);
     });
 
     it('should return the stringified error', () => {

public/app/core/utils/errors.ts

 import _ from 'lodash';
 
-export function getMessageFromError(err: any): string | null {
+export function getMessageFromError(err: string | (Error & { data?: any; statusText?: string })): string | null {
   if (err && !_.isString(err)) {
     if (err.message) {
       return err.message;
@@ -13,5 +13,5 @@ export function getMessageFromError(err: any): string | null {
     }
   }
 
-  return err;
+  return err as string;
 }

public/app/plugins/datasource/jaeger/QueryField.test.tsx

+import React from 'react';
+import { JaegerQueryField } from './QueryField';
+import { shallow } from 'enzyme';
+import { JaegerDatasource, JaegerQuery } from './datasource';
+import { ButtonCascader } from '@grafana/ui';
+
+describe('JaegerQueryField', function() {
+  it('shows empty value if no services returned', function() {
+    const dsMock: JaegerDatasource = {
+      metadataRequest(url: string) {
+        if (url.indexOf('/services') > 0) {
+          return Promise.resolve([]);
+        }
+        throw new Error(`Unexpected url: ${url}`);
+      },
+    } as any;
+
+    const wrapper = shallow(
+      <JaegerQueryField
+        history={[]}
+        datasource={dsMock}
+        query={{ query: '1234' } as JaegerQuery}
+        onRunQuery={() => {}}
+        onChange={() => {}}
+      />
+    );
+    expect(wrapper.find(ButtonCascader).props().options[0].label).toBe('No traces found');
+  });
+});

public/app/plugins/datasource/jaeger/QueryField.tsx

@@ -2,7 +2,8 @@ import React from 'react';
 import { JaegerDatasource, JaegerQuery } from './datasource';
 import { ButtonCascader, CascaderOption } from '@grafana/ui';
 
-import { ExploreQueryFieldProps } from '@grafana/data';
+import { AppEvents, ExploreQueryFieldProps } from '@grafana/data';
+import { appEvents } from '../../../core/core';
 
 const ALL_OPERATIONS_KEY = '__ALL__';
 const NO_TRACES_KEY = '__NO_TRACES__';
@@ -21,6 +22,8 @@ function getLabelFromTrace(trace: any): string {
 }
 
 export class JaegerQueryField extends React.PureComponent<Props, State> {
+  private _isMounted: boolean;
+
   constructor(props: Props, context: React.Context<any>) {
     super(props, context);
     this.state = {
@@ -29,16 +32,24 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
   }
 
   componentDidMount() {
+    this._isMounted = true;
     this.getServices();
   }
 
+  componentWillUnmount(): void {
+    this._isMounted = false;
+  }
+
   async getServices() {
     const url = '/api/services';
     const { datasource } = this.props;
     try {
-      const res = await datasource.metadataRequest(url);
-      if (res) {
-        const services = res as string[];
+      const services: string[] | null = await datasource.metadataRequest(url);
+      if (!this._isMounted) {
+        return;
+      }
+
+      if (services) {
         const serviceOptions: CascaderOption[] = services.sort().map(service => ({
           label: service,
           value: service,
@@ -47,7 +58,7 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
         this.setState({ serviceOptions });
       }
     } catch (error) {
-      console.error(error);
+      appEvents.emit(AppEvents.alertError, ['Failed to load services from Jaeger', error]);
     }
   }
 
@@ -56,6 +67,10 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
     if (selectedOptions.length === 1) {
       // Load operations
       const operations: string[] = await this.findOperations(service);
+      if (!this._isMounted) {
+        return;
+      }
+
       const allOperationsOption: CascaderOption = {
         label: '[ALL]',
         value: ALL_OPERATIONS_KEY,
@@ -85,6 +100,10 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
       const operationValue = selectedOptions[1].value;
       const operation = operationValue === ALL_OPERATIONS_KEY ? '' : operationValue;
       const traces: any[] = await this.findTraces(service, operation);
+      if (!this._isMounted) {
+        return;
+      }
+
       let traceOptions: CascaderOption[] = traces.map(trace => ({
         label: getLabelFromTrace(trace),
         value: trace.traceID,
@@ -128,7 +147,7 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
     try {
       return await datasource.metadataRequest(url);
     } catch (error) {
-      console.error(error);
+      appEvents.emit(AppEvents.alertError, ['Failed to load operations from Jaeger', error]);
     }
     return [];
   };
@@ -151,7 +170,7 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
     try {
       return await datasource.metadataRequest(url, traceSearch);
     } catch (error) {
-      console.error(error);
+      appEvents.emit(AppEvents.alertError, ['Failed to load traces from Jaeger', error]);
     }
     return [];
   };
@@ -168,12 +187,13 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
   render() {
     const { query, onChange } = this.props;
     const { serviceOptions } = this.state;
+    const cascaderOptions = serviceOptions && serviceOptions.length ? serviceOptions : noTracesFoundOptions;
 
     return (
       <>
         <div className="gf-form-inline gf-form-inline--nowrap">
           <div className="gf-form flex-shrink-0">
-            <ButtonCascader options={serviceOptions} onChange={this.onSelectTrace} loadData={this.onLoadOptions}>
+            <ButtonCascader options={cascaderOptions} onChange={this.onSelectTrace} loadData={this.onLoadOptions}>
               Traces
             </ButtonCascader>
           </div>
@@ -199,4 +219,15 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
   }
 }
 
+const noTracesFoundOptions = [
+  {
+    label: 'No traces found',
+    value: 'no_traces',
+    isLeaf: true,
+
+    // Cannot be disabled because then cascader shows 'loading' for some reason.
+    // disabled: true,
+  },
+];
+
 export default JaegerQueryField;

public/app/plugins/datasource/jaeger/datasource.ts

@@ -26,7 +26,7 @@ export class JaegerDatasource extends DataSourceApi<JaegerQuery> {
     super(instanceSettings);
   }
 
-  async metadataRequest(url: string, params?: Record<string, any>) {
+  async metadataRequest(url: string, params?: Record<string, any>): Promise<any> {
     const res = await this._request(url, params, { silent: true }).toPromise();
     return res.data.data;
   }