fix(dicomweb): handle missing and failed series metadata

src/store/dicom-web/dicom-meta-store.ts

@@ -49,6 +49,12 @@ interface State {
   studyPatient: Record<string, string>;
 }
 
+// The index has no slot for a series without its study, or an instance
+// without its series.
+const hasParentUids = (info: Instance) =>
+  (!info.SeriesInstanceUID || !!info.StudyInstanceUID) &&
+  (!info.SopInstanceUID || !!info.SeriesInstanceUID);
+
 export const useDicomMetaStore = defineStore('dicom-meta', {
   state: (): State => ({
     patientInfo: {},
@@ -64,7 +70,8 @@ export const useDicomMetaStore = defineStore('dicom-meta', {
     needsRebuild: {},
   }),
   actions: {
-    async importMeta(info: Instance) {
+    importMeta(info: Instance) {
+      if (!hasParentUids(info)) return;
       const patient = {
         PatientID: info.PatientID || ANONYMOUS_PATIENT_ID,
         PatientName: info.PatientName || ANONYMOUS_PATIENT,

src/store/dicom-web/dicom-web-store.ts

@@ -19,6 +19,7 @@ import {
   retrieveStudyMetadata,
   retrieveSeriesMetadata,
   parseUrl,
+  Instance,
 } from '@/src/core/dicom-web-api';
 import { useViewStore } from '@/src/store/views';
 
@@ -57,6 +58,20 @@ const levelToMetaKey = {
 
 type InitialDicomListFetchProgress = 'Idle' | 'Pending' | 'Done';
 
+// One failed request should not hide what the other requests returned.
+const importSettled = (results: PromiseSettledResult<Instance[]>[]) => {
+  const dicoms = useDicomMetaStore();
+  results.forEach((result) => {
+    if (result.status === 'fulfilled')
+      result.value.forEach((instance) => dicoms.importMeta(instance));
+  });
+  const failure = results.find((result) => result.status === 'rejected');
+  if (failure)
+    useMessageStore().addError('Failed to fetch DICOMWeb metadata', {
+      error: failure.reason,
+    });
+};
+
 /**
  * Collect DICOM data from DICOMWeb
  */
@@ -111,7 +126,7 @@ export const useDicomWebStore = defineStore('dicom-web', () => {
 
   const fetchPatientMeta = async (patientKey: string) => {
     const dicoms = useDicomMetaStore();
-    const series = await Promise.all(
+    const series = await Promise.allSettled(
       dicoms.patientStudies[patientKey]
         .map((studyKey) => dicoms.studyInfo[studyKey])
         .map(async (studyMeta) => {
@@ -124,12 +139,12 @@ export const useDicomWebStore = defineStore('dicom-web', () => {
           }));
         })
     );
-    series.flat().forEach((instance) => dicoms.importMeta(instance));
+    importSettled(series);
   };
 
   const fetchVolumesMeta = async (volumeKeys: string[]) => {
     const dicoms = useDicomMetaStore();
-    const series = await Promise.all(
+    const series = await Promise.allSettled(
       volumeKeys.map(async (volumeKey) => {
         const volumeMeta = dicoms.volumeInfo[volumeKey];
         const studyMeta = dicoms.studyInfo[dicoms.volumeStudy[volumeKey]];
@@ -144,7 +159,7 @@ export const useDicomWebStore = defineStore('dicom-web', () => {
         }));
       })
     );
-    series.flat().forEach((instance) => dicoms.importMeta(instance));
+    importSettled(series);
   };
 
   const volumes = ref({} as Progress);
@@ -161,12 +176,6 @@ export const useDicomWebStore = defineStore('dicom-web', () => {
       total: 0,
     };
 
-    const { SeriesInstanceUID: seriesInstanceUID } =
-      dicoms.volumeInfo[volumeKey];
-    const studyKey = dicoms.volumeStudy[volumeKey];
-    const { StudyInstanceUID: studyInstanceUID } = dicoms.studyInfo[studyKey];
-    const seriesInfo = { studyInstanceUID, seriesInstanceUID };
-
     const progressCallback = ({ loaded, total }: ProgressEvent) => {
       volumes.value[volumeKey] = {
         ...volumes.value[volumeKey],
@@ -176,6 +185,15 @@ export const useDicomWebStore = defineStore('dicom-web', () => {
     };
 
     try {
+      const volumeInfo = dicoms.volumeInfo[volumeKey];
+      if (!volumeInfo) {
+        throw new Error('DICOMWeb metadata is unavailable for this series');
+      }
+      const studyInfo = dicoms.studyInfo[dicoms.volumeStudy[volumeKey]];
+      const seriesInfo = {
+        studyInstanceUID: studyInfo.StudyInstanceUID,
+        seriesInstanceUID: volumeInfo.SeriesInstanceUID,
+      };
       const files = await fetchSeries(
         cleanHost.value,
         seriesInfo,
@@ -247,16 +265,16 @@ export const useDicomWebStore = defineStore('dicom-web', () => {
         ...fetchedMeta,
       }));
       fullMeta.forEach((instance) => dicoms.importMeta(instance));
+
+      // downloadVolume reports its own failures, so the list need not wait.
+      if (deepestLevel === 'series' && fullMeta.length > 0) {
+        downloadVolume(Object.values(parsedURL.value).pop() as string);
+      }
     } catch (error) {
       fetchError.value = error;
       console.error(error);
     }
 
-    if (deepestLevel === 'series') {
-      const seriesID = Object.values(parsedURL.value).pop() as string;
-      downloadVolume(seriesID);
-    }
-
     fetchDicomsProgress.value = 'Done';
   };
 

tests/pageobjects/volview.page.ts

@@ -92,6 +92,15 @@ class VolViewPage extends Page {
     );
   }
 
+  async getNotificationTitles() {
+    await this.notifications.click();
+    const titles = $$(
+      '.message-center .v-expansion-panel-title .header > span'
+    );
+    await titles[0].waitForDisplayed();
+    return titles.map((title) => title.getText());
+  }
+
   async selectTool(icon: string) {
     const button = $(`button.tool-btn i[class~=${icon}]`);
     await button.waitForClickable();

tests/server/dicomweb-server.ts

@@ -0,0 +1,86 @@
+import express from 'express';
+import cors from 'cors';
+import type { Server } from 'http';
+import type { AddressInfo } from 'net';
+
+export type Reply = {
+  status: number;
+  json?: unknown;
+  dicoms?: Uint8Array[];
+};
+
+export type DicomWebReplies = {
+  studySearch: Reply;
+  seriesSearch: Reply;
+  seriesMetadata: Reply;
+  seriesRetrieval: Reply;
+};
+
+const BOUNDARY = 'volview-e2e-boundary';
+
+// WADO-RS answers a series retrieval with one multipart/related part per instance.
+const multipartRelated = (dicoms: Uint8Array[]) =>
+  Buffer.concat([
+    ...dicoms.flatMap((dicom) => [
+      Buffer.from(`--${BOUNDARY}\r\nContent-Type: application/dicom\r\n\r\n`),
+      Buffer.from(dicom),
+      Buffer.from('\r\n'),
+    ]),
+    Buffer.from(`--${BOUNDARY}--`),
+  ]);
+
+const send = (res: express.Response, reply: Reply) => {
+  if (reply.dicoms) {
+    res
+      .status(reply.status)
+      .set(
+        'Content-Type',
+        `multipart/related; type="application/dicom"; boundary=${BOUNDARY}`
+      )
+      .send(multipartRelated(reply.dicoms));
+    return;
+  }
+  res.status(reply.status).json(reply.json ?? null);
+};
+
+/**
+ * Serves the DICOMweb routes VolView requests for a linked study or series,
+ * answering each with whatever the spec last assigned to `replies`.
+ */
+export async function startDicomWebServer() {
+  const replies: DicomWebReplies = {
+    studySearch: { status: 404 },
+    seriesSearch: { status: 404 },
+    seriesMetadata: { status: 404 },
+    seriesRetrieval: { status: 404 },
+  };
+
+  const app = express();
+  app.use(cors());
+  app.get('/dicom-web/studies', (_req, res) => send(res, replies.studySearch));
+  app.get('/dicom-web/studies/:study/series', (_req, res) =>
+    send(res, replies.seriesSearch)
+  );
+  app.get('/dicom-web/studies/:study/series/:series/metadata', (_req, res) =>
+    send(res, replies.seriesMetadata)
+  );
+  app.get('/dicom-web/studies/:study/series/:series', (_req, res) =>
+    send(res, replies.seriesRetrieval)
+  );
+
+  // Port 0 so specs in parallel workers never contend for the shared AUX_PORT.
+  const server = await new Promise<Server>((resolve) => {
+    const listening = app.listen(0, () => resolve(listening));
+  });
+  const { port } = server.address() as AddressInfo;
+
+  return {
+    replies,
+    root: `http://localhost:${port}/dicom-web`,
+    stop: () =>
+      new Promise<void>((resolve) => {
+        server.closeAllConnections();
+        server.close(() => resolve());
+      }),
+  };
+}

tests/specs/dicomweb-metadata-failures.e2e.ts

@@ -0,0 +1,143 @@
+import { volViewPage } from '../pageobjects/volview.page';
+import { startDicomWebServer } from '../server/dicomweb-server';
+import { buildSyntheticDicom, newUid } from './syntheticDicom';
+
+const SLICE_COUNT = 3;
+
+const tag = (vr: string, value: unknown) => ({ vr, Value: [value] });
+
+// A QIDO-RS series search result in the DICOM JSON model.
+const seriesRecord = (studyUid: string, seriesUid: string) => ({
+  '00100010': tag('PN', { Alphabetic: 'DICOMWeb^Patient' }),
+  '00100020': tag('LO', 'dicomweb-patient'),
+  '0020000D': tag('UI', studyUid),
+  '0020000E': tag('UI', seriesUid),
+  '00080060': tag('CS', 'MR'),
+  '0008103E': tag('LO', 'Linked series'),
+});
+
+const makeSeries = (studyUid = newUid()) => {
+  const seriesUid = newUid();
+  const sopUids = Array.from({ length: SLICE_COUNT }, () => newUid());
+  return {
+    studyUid,
+    seriesUid,
+    search: [seriesRecord(studyUid, seriesUid)],
+    metadata: sopUids.map((sopUid, i) => ({
+      ...seriesRecord(studyUid, seriesUid),
+      '00080018': tag('UI', sopUid),
+      '00200013': tag('IS', String(i + 1)),
+      '00280010': tag('US', 4),
+      '00280011': tag('US', 4),
+    })),
+    dicoms: sopUids.map((sopUid, i) =>
+      buildSyntheticDicom({
+        studyUid,
+        seriesUid,
+        sopUid,
+        instanceNumber: i + 1,
+        imageOrientationPatient: [1, 0, 0, 0, 1, 0],
+        imagePositionPatient: [0, 0, i],
+      })
+    ),
+  };
+};
+
+const dicomWebMessage = () => $('#data-module p.error-message');
+
+describe('DICOMWeb metadata failures', () => {
+  let server: Awaited<ReturnType<typeof startDicomWebServer>>;
+
+  const linkTo = (...path: string[]) =>
+    volViewPage.open(`?dicomweb=${[server.root, ...path].join('/')}`);
+
+  const linkToSeries = ({
+    studyUid,
+    seriesUid,
+  }: ReturnType<typeof makeSeries>) =>
+    linkTo('studies', studyUid, 'series', seriesUid);
+
+  beforeEach(async () => {
+    server = await startDicomWebServer();
+  });
+
+  afterEach(async () => {
+    await server.stop();
+  });
+
+  it('loads a linked series once its failed metadata request is retried', async () => {
+    const series = makeSeries();
+    server.replies.seriesMetadata = { status: 500 };
+    await linkToSeries(series);
+
+    await expect(dicomWebMessage()).toHaveText(
+      expect.stringContaining('Error fetching DICOMWeb data')
+    );
+    expect(await volViewPage.getNotificationsCount()).toBe(0);
+
+    server.replies.seriesSearch = { status: 200, json: series.search };
+    server.replies.seriesMetadata = { status: 200, json: series.metadata };
+    server.replies.seriesRetrieval = { status: 200, dicoms: series.dicoms };
+    // Closing Settings unmounts the DICOMWeb fields, which retries a failed fetch.
+    await volViewPage.openSettings();
+    await volViewPage.closeSettings();
+
+    await $('.volume-card .mdi-check').waitForDisplayed();
+    await volViewPage.waitForViews();
+    expect(await volViewPage.getNotificationsCount()).toBe(0);
+  });
+
+  it('reports a linked series with no metadata without starting a download', async () => {
+    server.replies.seriesMetadata = { status: 200, json: [] };
+    await linkToSeries(makeSeries());
+
+    await expect(dicomWebMessage()).toHaveText('Found zero dicoms.');
+    expect(await volViewPage.getNotificationsCount()).toBe(0);
+  });
+
+  it('reports a linked series that its metadata does not describe', async () => {
+    const linked = makeSeries();
+    const other = makeSeries(linked.studyUid);
+    server.replies.seriesSearch = { status: 200, json: other.search };
+    server.replies.seriesMetadata = { status: 200, json: other.metadata };
+    await linkToSeries(linked);
+
+    await volViewPage.waitForNotification();
+    expect(await volViewPage.getNotificationTitles()).toEqual([
+      'Failed to load DICOM',
+    ]);
+  });
+
+  it('lists the studies of a search that also returns a record without a study UID', async () => {
+    const { studyUid, seriesUid } = makeSeries();
+    server.replies.studySearch = {
+      status: 200,
+      json: [
+        seriesRecord(studyUid, seriesUid),
+        {
+          '00100010': tag('PN', { Alphabetic: 'Malformed^Record' }),
+          '0020000E': tag('UI', newUid()),
+        },
+      ],
+    };
+    await linkTo();
+
+    await expect($('#data-module .patient-header-name')).toHaveText(
+      'DICOMWeb^Patient'
+    );
+  });
+
+  it('stops the series spinner when browsing a study whose series metadata fails', async () => {
+    const series = makeSeries();
+    server.replies.seriesSearch = { status: 200, json: series.search };
+    server.replies.seriesMetadata = { status: 500 };
+    await linkTo('studies', series.studyUid);
+
+    await volViewPage.waitForNotification();
+    await expect($('.volume-list')).toExist();
+    await expect($('.volume-list .fetching')).not.toExist();
+    expect(await volViewPage.getNotificationTitles()).toEqual([
+      'Failed to fetch DICOMWeb metadata',
+    ]);
+  });
+});