loki: query chunking: better error handling (#64811)

* loki: query chunking: better error handling

* better comment text

Co-authored-by: Matias Chomicki <matyax@gmail.com>

* better comment text

Co-authored-by: Matias Chomicki <matyax@gmail.com>

* changed merge approach

* simplified code

* fixed test

* removed forgotten code

---------

Co-authored-by: Matias Chomicki <matyax@gmail.com>

public/app/plugins/datasource/loki/querySplitting.test.ts

@@ -42,7 +42,7 @@ describe('runPartitionedQueries()', () => {
       .spyOn(datasource, 'runQuery')
       .mockReturnValue(of({ state: LoadingState.Error, error: { refId: 'A', message: 'Error' }, data: [] }));
     await expect(runPartitionedQueries(datasource, request)).toEmitValuesWith((values) => {
-      expect(values).toEqual([{ refId: 'A', message: 'Error' }]);
+      expect(values).toEqual([{ error: { refId: 'A', message: 'Error' }, data: [], state: LoadingState.Streaming }]);
     });
   });
 

public/app/plugins/datasource/loki/querySplitting.ts

@@ -131,10 +131,10 @@ export function runGroupedQueries(datasource: LokiDatasource, requests: LokiGrou
       .runQuery({ ...requests[requestGroup].request, range, requestId, targets })
       .subscribe({
         next: (partialResponse) => {
-          if (partialResponse.error) {
-            subscriber.error(partialResponse.error);
-          }
           mergedResponse = combineResponses(mergedResponse, partialResponse);
+          if ((mergedResponse.errors ?? []).length > 0 || mergedResponse.error != null) {
+            shouldStop = true;
+          }
         },
         complete: () => {
           subscriber.next(mergedResponse);

public/app/plugins/datasource/loki/queryUtils.test.ts

@@ -476,6 +476,75 @@ describe('combineResponses', () => {
     expect(combineResponses(null, responseB)).not.toBe(responseB);
   });
 
+  it('combine when first param has errors', () => {
+    const { metricFrameA, metricFrameB } = getMockFrames();
+    const errorA = {
+      message: 'errorA',
+    };
+    const responseA: DataQueryResponse = {
+      data: [metricFrameA],
+      error: errorA,
+      errors: [errorA],
+    };
+    const responseB: DataQueryResponse = {
+      data: [metricFrameB],
+    };
+
+    const combined = combineResponses(responseA, responseB);
+    expect(combined.data[0].length).toBe(4);
+    expect(combined.error?.message).toBe('errorA');
+    expect(combined.errors).toHaveLength(1);
+    expect(combined.errors?.[0]?.message).toBe('errorA');
+  });
+
+  it('combine when second param has errors', () => {
+    const { metricFrameA, metricFrameB } = getMockFrames();
+    const responseA: DataQueryResponse = {
+      data: [metricFrameA],
+    };
+    const errorB = {
+      message: 'errorB',
+    };
+    const responseB: DataQueryResponse = {
+      data: [metricFrameB],
+      error: errorB,
+      errors: [errorB],
+    };
+
+    const combined = combineResponses(responseA, responseB);
+    expect(combined.data[0].length).toBe(4);
+    expect(combined.error?.message).toBe('errorB');
+    expect(combined.errors).toHaveLength(1);
+    expect(combined.errors?.[0]?.message).toBe('errorB');
+  });
+
+  it('combine when both params have errors', () => {
+    const { metricFrameA, metricFrameB } = getMockFrames();
+    const errorA = {
+      message: 'errorA',
+    };
+    const errorB = {
+      message: 'errorB',
+    };
+    const responseA: DataQueryResponse = {
+      data: [metricFrameA],
+      error: errorA,
+      errors: [errorA],
+    };
+    const responseB: DataQueryResponse = {
+      data: [metricFrameB],
+      error: errorB,
+      errors: [errorB],
+    };
+
+    const combined = combineResponses(responseA, responseB);
+    expect(combined.data[0].length).toBe(4);
+    expect(combined.error?.message).toBe('errorA');
+    expect(combined.errors).toHaveLength(2);
+    expect(combined.errors?.[0]?.message).toBe('errorA');
+    expect(combined.errors?.[1]?.message).toBe('errorB');
+  });
+
   describe('combine stats', () => {
     const { metricFrameA } = getMockFrames();
     const makeResponse = (stats?: QueryResultMetaStat[]): DataQueryResponse => ({

public/app/plugins/datasource/loki/queryUtils.ts

@@ -335,6 +335,22 @@ export function combineResponses(currentResult: DataQueryResponse | null, newRes
     combineFrames(currentFrame, newFrame);
   });
 
+  const mergedErrors = [...(currentResult.errors ?? []), ...(newResult.errors ?? [])];
+
+  // we make sure to have `.errors` as undefined, instead of empty-array
+  // when no errors.
+
+  if (mergedErrors.length > 0) {
+    currentResult.errors = mergedErrors;
+  }
+
+  // the `.error` attribute is obsolete now,
+  // but we have to maintain it, otherwise
+  // some grafana parts do not behave well.
+  // we just choose the old error, if it exists,
+  // otherwise the new error, if it exists.
+  currentResult.error = currentResult.error ?? newResult.error;
+
   return currentResult;
 }