Project: doctolib-doctolib · Branch: .invalid · Mode: ` · **Model:**claude-opus-4-7`
Stats: 1 user messages, 39 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Review the unstaged and staged changes on branch fhir-2215_wire-dmp-errors-through-dmpnhradapter in /Users/USER/workspace/doctolib/doctolib.
The PR is https://github.com/doctolib/doctolib/pull/240721
The changed files are:
- packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts — updated toDocumentSearchResult to include status: 'success', updated searchDocuments to return error-as-data instead of throwing, uses DmpApiError to assemble error message and propagate code
- packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts — updated tests to expect status: 'success'/'error' in results, added test for API error scenario, uses jest.requireActual for real DmpApiError class
- packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts — propagates error-as-data through React Query, wraps fetchQuery in try/catch for network errors, adds status: 'success' to all success paths
- packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.test.ts — updated all expected return values to include status, added test for adapter error passthrough
- packages/@doctolib/national-health-records/src/variants/types.ts — imports SearchDocumentsResult from @doctolib/tpf-national-health-records-ui instead of defining it locally
- packages/@doctolib/national-health-records/package.json — bumped tpf-national-health-records-ui to ^15.1.0
- packages/@doctolib/shared_patient_file_and_appointment_page_components/package.json — bumped tpf-national-health-records-ui to ^15.1.0
- package.json and package-lock.json — updated to resolve <REDACTED> and <REDACTED>
Focus your review on:
1. Correctness of the error-as-data propagation pattern
2. Whether the retry logic in useSearchDocuments correctly handles: (a) adapter-returned errors (no retry), (b) DMPAccessDeniedByExcededThreshold thrown errors (no retry), (c) network errors (3 retries)
3. TypeScript types correctness — especially NhrDocumentSearchResult | null returned from queryFn
4. Test completeness and correctness
5. Code style adherence per CLAUDE.md
Turn 1 — Assistant
I'll review the changes on this branch. Let me start by exploring the relevant files.
Tools used:
Bash:git branch --show-current && git status
Tool results:
→ result (ok): fhir-2215_wire-dmp-errors-through-dmpnhradapter On branch fhir-2215_wire-dmp-errors-through-dmpnhradapter Your branch is up to date with 'origin/fhir-2215_wire-dmp-errors-through-dmpnhradapter'.
It took 2.51 seconds to enumerate untracked files. See 'git help status' for information on how to improve this.
nothing to commit, working tree clean
I'll conduct a read-only code review of the changes. Let me examine the modified files.
Tools used:
Bash:git log --oneline master..HEAD 2>/dev/null | head -20
Tool results:
→ result (ok): 3300dbbdd290 feat(FHIR-2215): wire DMP errors through DmpNhrAdapter and useSearchDocuments
Tools used:
Bash:git diff master...HEAD --stat 2>/dev/null
Tool results:
→ result (ok): package-lock.json | 176 ++++++++++++++++++--- package.json | 2 +- .../@doctolib/national-health-records/package.json | 2 +- .../src/business/adapters/DmpNhrAdapter.test.ts | 62 +++++--- .../src/business/adapters/DmpNhrAdapter.ts | 18 ++- .../src/hooks/useSearchDocuments.test.ts | 64 ++++++-- .../src/hooks/useSearchDocuments.ts | 61 +++---- .../national-health-records/src/vari …(truncated, 137 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts
Tool results:
→ result (ok): 1 import type { 2 DmpApi, 3 DmpAuthor, 4 DmpDocument, 5 DmpDocumentContent, 6 DocumentStatusUpdateProps, 7 SearchProps, 8 SearchResult as DmpSearchResult, 9 } from '@doctolib/dmp' 10 import { DmpApiError, DOCUMENT_STATUS, DOCUMENT_VISIBILITY, getPatientInsForDMP } from '@doctolib/dmp' 11 import { isUnstructured } from '@doctolib/cda/types/UnstructuredBody' 12 import { isStructured } from '@doctolib/cda/types/StructuredBody' 13 import type { Patient } from 'monolith_DEPRECATED/types …(truncated, 9256 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts
Tool results:
→ result (ok): 1 import { getPatientInsForDMP } from '@doctolib/dmp'
2 import type { DmpApi, DmpApiErrorInfo, DmpDocumentContent, SearchResult as DmpSearchResult } from '@doctolib/dmp'
3 import type { NhrDocumentSearchProps } from '@doctolib/tpf-national-health-records'
4 import { DmpNhrAdapter } from './DmpNhrAdapter'
5
6 jest.mock('@doctolib/dmp', () => {
7 const actual = jest.requireActual<typeof import('@doctolib/dmp')>('@doctolib/dmp')
8 return {
9 ...actual,
10 getPatientInsForDMP: jest.fn()
…(truncated, 21951 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts
Tool results:
→ result (ok): 1 import { useQueryClient } from '@tanstack/react-query-v5' 2 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient' 3 import { isFeatureEnabled } from '@doctolib/variation_management' 4 import type { NhrDocumentListVariant, MapFiltersToSearchDocumentsParams, ServerFilters } from '../variants/types' 5 import { useNationalHealthRecords } from './useNationalHealthRecords' 6 import { generateNhrDocuments } from '../mocks/generateNhrDocuments' 7 import { filterMockedDocuments } …(truncated, 2503 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.test.ts
Tool results:
→ result (ok): 1 import React from 'react' 2 import { renderHook } from '@testing-library/react' 3 import { QueryClient, QueryClientProvider } from '@tanstack/react-query-v5' 4 import * as variationManagement from '@doctolib/variation_management' 5 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient' 6 import type { NhrDocument, NhrDocumentSearchParams } from '@doctolib/tpf-national-health-records' 7 import { useNationalHealthRecords } from './useNationalHealthRecords' 8 import { generat …(truncated, 15588 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/types.ts
Tool results:
→ result (ok): 1 import type { ColumnConfig, FilterConfig, SearchDocumentsResult } from '@doctolib/tpf-national-health-records-ui'
2 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient'
3 import type { NhrDocumentSearchParams } from '@doctolib/tpf-national-health-records'
4
5 export type { SearchDocumentsResult }
6
7 export type NhrDocumentListVariant = {
8 columns: ColumnConfig[]
9 filters: FilterConfig[]
10 searchDocuments: (appliedFilters?: Record<string, unknown>) => Promise<
…(truncated, 820 more chars)
Now let me look at the DmpApiError class and the SearchResult type to verify the error flow is correct.
Tools used:
Bash:find /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp -type f \( -name "*.ts" -not -name "*.test.ts" \) | head -40
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/index.ts /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/test-index.ts /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/types/FrDmpTypes.ts /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/types/DmpApi.ts /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/business/frDmpPayloadBuilders.ts /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/busine …(truncated, 2759 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/api/DmpApiError.ts
Tool results:
→ result (ok): 1 import type { DmpApiErrorInfo } from '../types/DmpApi'
2
3 export class DmpApiError extends Error {
4 readonly apiErrorCode: string | undefined
5
6 readonly apiErrorType: string | undefined
7
8 readonly apiErrorContext: string | undefined
9
10 readonly apiErrorDescription: string | undefined
11
12 readonly apiErrorExtendedInformations: string | undefined
13
14 readonly apiErrorTypeDescription: string | undefined
15
16 readonly status: string
17
18 constructor(info: DmpA
…(truncated, 730 more chars)
Tools used:
Bash:grep -n "DmpApiErrorInfo\|SearchResult" /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/types/DmpApi.ts | head -20
Tool results:
→ result (ok): 108: search: (props: SearchProps) => Promise<SearchResult> 170:export interface DmpApiErrorInfo { 180:export interface SearchResult { 182: error?: DmpApiErrorInfo
Tools used:
Bash:grep -n "^export\|DmpApiError\b\|SearchResult\b" /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/index.ts | head -30
Tool results:
→ result (ok): 1:export { 9:export { getErrorMessage, getWarningMessage } from './business/DmpI18nMessage' 10:export type { 19: SearchResult, 22:export { DmpApiImpl } from './business/DmpApi' 25:export { 41:export type { 50:export { useDmpSetUp, type DmpSetUpResult } from './hooks/useDmpSetUp' 51:export { useDmpConsentService } from './hooks/useDmpConsentService' 52:export { useDmpGetConsent } from './hooks/useDmpGetConsent' 53:export { DmpSharingService, type DmpSharingOptionsInput } from './hooks/DmpSharing …(truncated, 969 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/types/DmpApi.ts(168–188)
Tool results:
→ result (ok): 168 } 169 170 export interface DmpApiErrorInfo { 171 apiErrorCode?: string 172 apiErrorType?: string 173 apiErrorContext?: string 174 apiErrorDescription?: string 175 apiErrorExtendedInformations?: string 176 apiErrorTypeDescription?: string 177 status: string 178 } 179 180 export interface SearchResult { 181 documents: DmpDocument[] 182 error?: DmpApiErrorInfo 183 } 184 185 export type GetDocumentsContentProps = Omit<GetDocumentsContentParams, 'sessionId'> 186 187 export t …(truncated, 33 more chars)
Now let me look at the NhrDocumentSearchResult type in @doctolib/tpf-national-health-records to verify the status: 'error' typing.
Tools used:
Bash:find /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records -name "*.d.ts" -type f 2>/dev/null | head -10
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records/dist/index.d.ts /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records/dist/business/nhr-service-factory.d.ts /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records/dist/business/types/api.d.ts /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records/dist/business/types/service.d.ts /Users/USER/workspa …(truncated, 770 more chars)
Tools used:
Bash:grep -rn "NhrDocumentSearchResult\|NhrResultStatus" /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records/dist/ 2>/dev/null | head -20
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records/dist/index.d.ts:2:export type { NhrResult, NhrResultStatus, NhrDocumentSearchParams, NhrDocumentSearchProps, NhrDocumentSearchResult, NhrGetDocumentsContentParams, NhrGetDocumentsContentProps, NhrDocumentContentResult, NhrDocumentMetadataUpdatePayload, NhrDocumentMetadataUpdateParams, NhrDocumentMetadataUpdateAdditionalInfo, NhrDocumentMetadataUpdateProps, NhrDocumentMetadataUpdatePayloadResult, NhrDocumen …(truncated, 2735 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records/dist/business/types/api.d.ts
Tool results:
→ result (ok): 1 import type { DateRange, NhrDocument, NhrDocumentFile } from '#business/types/document'; 2 import type { NhrPatientInformation } from '#business/types/participant'; 3 export type NhrResultStatus = 'success' | 'skipped' | 'error' | 'partial success'; 4 export type NhrResult = { 5 status: NhrResultStatus; 6 message?: string; 7 code?: string; 8 }; 9 export type NhrDocumentSearchParams = { 10 formats?: string[]; 11 categories?: string[]; 12 status?: string; 13 serviceSt …(truncated, 1993 more chars)
Now let me look at SearchDocumentsResult from @doctolib/tpf-national-health-records-ui to verify the type flow.
Tools used:
Bash:grep -rn "SearchDocumentsResult" /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records-ui/dist/ 2>/dev/null | head -10
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records-ui/dist/types.d.ts:47:export type SearchDocumentsResult = { /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records-ui/dist/types.d.ts:54:export type SearchDocumentsFn = (appliedFilters?: Record<string, unknown>) => Promise<SearchDocumentsResult>; /Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records-ui/dist/index.d.ts:3:export type { …(truncated, 322 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/node_modules/@doctolib/tpf-national-health-records-ui/dist/types.d.ts(40–65)
Tool results:
→ result (ok): 40 / One filter control definition — render patches only its own key(s) via onChange */ 41 export type FilterConfig = { 42 key: string; 43 render: ( 44 / generic — DMP variant casts to DmpServerFilters */ 45 filters: Record<string, unknown>, onChange: (patch: Record<string, unknown>) => void, ctx: FilterRenderContext) => ReactNode; 46 }; 47 export type SearchDocumentsResult = { 48 data: NhrDocument[]; 49 total?: number; 50 status: NhrResultStatus; 51 message?: …(truncated, 614 more chars)
Good. Now let me check the useNationalHealthRecords hook and make sure the returned adapter error also includes DmpApiErrorInfo fields (e.g., code) for proper retry-decision propagation:
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/hooks/useNationalHealthRecords.ts
Tool results:
→ result (ok): 1 import type {
2 NhrDocumentSearchProps,
3 NhrDocumentSearchResult,
4 NhrGetDocumentsContentProps,
5 NhrDocumentContentResult,
6 NhrDocumentMetadataUpdateProps,
7 NhrDocumentMetadataUpdateResult,
8 } from '@doctolib/tpf-national-health-records'
9 import { useProductLineVariation, ProductLineKey } from '@doctolib/variation_management'
10 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient'
11 import { useDmpNhr } from './useDmpNhr'
12
13 export type UseNhrOper
…(truncated, 966 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/package.json
Tool results:
→ result (ok): 1 {
2 "name": "@doctolib/national-health-records",
3 "version": "1.0.0",
4 "publishConfig": {
5 "registry": "https://doctolib-580698825394.d.codeartifact.eu-west-1.amazonaws.com/npm/npm-internal/"
6 },
7 "repository": {
8 "type": "git",
9 "url": "git+https://github.com/doctolib/doctolib.git"
10 },
11 "keywords": [
12 "upfront",
13 "modular frontend",
14 "in-monolith-package"
15 ],
16 "engines": {
17 "node": ">= 24.8.0",
18 "npm": ">= 11.6.0"
19
…(truncated, 1095 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/shared_patient_file_and_appointment_page_components/package.json
Tool results:
→ result (ok): 1 { 2 "name": "@doctolib/shared_patient_file_and_appointment_page_components", 3 "version": "1.0.0", 4 "license": "Doctolib", 5 "scripts": { 6 "test": "npm run test:unit", 7 "test:unit": "jest --passWithNoTests", 8 "lint": "eslint ." 9 }, 10 "dependencies": { 11 "@doctolib/clinical_contact_list": "^1.0.0", 12 "@doctolib/cost-bearer-plan": "^1.0.0", 13 "@doctolib/de_billing_reference_sdk": "^1.0.0", 14 "@doctolib/de_public_billing": "^1.0.0", 15 "@docto …(truncated, 5017 more chars)
Now let me look at the diff for useSearchDocuments to see what changed there, specifically the React Query retry logic and error handling patterns.
Tools used:
Bash:git diff master...HEAD -- packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts
Tool results:
→ result (ok): diff --git a/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts b/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts index c3b3f77ca0bc..a81db371cf9e 100644 --- a/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts +++ b/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts @@ -19,14 +19,6 @@ const useSearchDocuments = ({ patient, mapFiltersToSearchDocumentsParams, }: UseSearchDocumentsProps): NhrD …(truncated, 3176 more chars)
Tools used:
Bash:git diff master...HEAD -- packages/@doctolib/national-health-records/src/variants/types.ts
Tool results:
→ result (ok): diff --git a/packages/@doctolib/national-health-records/src/variants/types.ts b/packages/@doctolib/national-health-records/src/variants/types.ts index 01ab5a8e575a..27e07b9433c1 100644 --- a/packages/@doctolib/national-health-records/src/variants/types.ts +++ b/packages/@doctolib/national-health-records/src/variants/types.ts @@ -1,11 +1,8 @@ -import type { ColumnConfig, FilterConfig } from '@doctolib/tpf-national-health-records-ui' +import type { ColumnConfig, FilterConfig, SearchDocumentsResult …(truncated, 500 more chars)
Now I have enough context. Let me also verify the DmpApi.ts error handling and how DmpApiError is assembled when searching.
Tools used:
Bash:grep -n "error\|DmpApiError" /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/business/DmpApi.ts | head -30
Tool results:
→ result (ok): 122: 1: 'error', 169: private emitSharingError(sharingSessionId: string, error: Error) { 170: this.props.onError(error) 171: dispatchDmpSharingError({ sharingSessionId, error }) 179: private emitAuthError(error: Error) { 180: this.props.onError(error) 181: dispatchDmpAuthError({ error }) 272: // Legacy does not throw on GetDocumentsContent errors — it returns 389: // Legacy does not throw on delete errors — it continues silently 517: error: 606: // Le …(truncated, 771 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/business/DmpApi.ts(500–560)
Tool results:
→ result (ok): 500 serviceStartDate: doc.s_serviceStartDate, 501 serviceStopDate: doc.s_serviceStopDate, 502 healthCareFacilityTypeCode: doc.s_healthCareFacilityTypeCode, 503 practiceSettingCode: doc.s_practiceSettingCode, 504 submissionDate: doc.s_submissionDate, 505 submissionSetId: doc.s_submissionSetId, 506 uuid: doc.s_uuid, 507 previousUuid: doc.s_previousUuid, 508 nextUuid: doc.s_nextUuid, 509 documentStatus: doc.i_document_Status, 510 doc …(truncated, 2102 more chars)
Now let me check if i_apiErrorCode is a string or number. The type says string but that might be mismatched:
Tools used:
Bash:grep -n "i_apiErrorCode\|s_apiErrorCode" /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src -r | head -20
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/business/FrDmpCommunication.ts:77: response: { s_status, i_apiErrorCode, i_apiErrorType } = {}, /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/business/FrDmpCommunication.ts:99: i_apiErrorCode, /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/types/FrDmpTypes.ts:196: i_apiErrorCode?: string /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/types/FrDmpTypes.ts:416: / …(truncated, 1372 more chars)
Good, apiErrorCode is a string. Now let me see the diff for DmpNhrAdapter.ts to verify the error handling:
Tools used:
Bash:git diff master...HEAD -- packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts
Tool results:
→ result (ok): diff --git a/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts b/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts index 8dd3bbcea9f8..d8222903a832 100644 --- a/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts +++ b/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts @@ -7,7 +7,7 @@ import type { SearchProps, SearchResult as DmpSearchResult, } from '@doctolib/dmp' …(truncated, 1663 more chars)
Now let me look at the DmpApiErrorInfo fields and verify how errors propagate through retries:
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts(36–66)
Tool results:
→ result (ok): 36 try { 37 const result = await queryClient.fetchQuery({ 38 queryKey: ['dmp-documents', ins, appliedFilters], 39 queryFn: async () => 40 (await nhrOperations.searchDocuments({ 41 patient: toNhrPatientInformation(ins), 42 params: mapFiltersToSearchDocumentsParams(appliedFilters as ServerFilters), 43 })) ?? null, 44 staleTime: DMP_DOCUMENTS_STALE_TIME_MS, 45 retry: (failureCount, error) => { 46 if (err …(truncated, 696 more chars)
Now I have a thorough understanding. Let me verify a few more things about the types and retry behavior before forming my review.
- The
queryFnnow returnsNhrDocumentSearchResult | null. React Query v5 forbidsundefined, so?? nullis required. This is fine. - The adapter no longer throws for "patient_national_health_identifier_required" - it returns error-as-data.
- The
retrycallback checkserror.message.includes('DMPAccessDeniedByExcededThreshold'). This only triggers when the adapter THROWS. But now, the adapter returns a result withstatus: 'error'andcode: 'DMPAccessDeniedByExcededThreshold', not throws. So the retry logic path(b) DMPAccessDeniedByExcededThreshold thrown errors (no retry)only covers thrown errors from other parts of the adapter - but DmpApiError is NOT thrown anymore in searchDocuments.
This is concerning! Let me examine more carefully. Looking at the test file, I see that the existing test "does not retry on DMPAccessDeniedByExcededThreshold error" uses mockSearchDocuments.mockRejectedValue(error) — it tests a thrown error case. But now that the adapter returns data instead of throwing, would this path ever be hit in practice?
Let me check if any other async code in the DmpApi chain could throw with a DMPAccessDeniedByExcededThreshold-containing message:
Tools used:
Bash:grep -rn "DMPAccessDeniedByExcededThreshold" /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/ 2>/dev/null | head -10
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:grep -rn "throw\|reject" /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/business/DmpApi.ts | grep -i "search\|findDocument" | head -10
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/business/DmpApi.ts:606: // Legacy does not throw on FindDocuments errors — it returns
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/dmp/src/business/DmpApi.ts(595–635)
Tool results:
→ result (ok): 595 })) 596 597 private async find(props: SearchProps): Promise<SearchResult> { 598 if (this.dmpService) { 599 const payload = buildFindDocumentsPayload(props) 600 601 try { 602 const response = await this.dmpService.call<FindDocumentsResponse>(DmpAction.FindDocuments, payload) 603 return this.toSearchResult(response) 604 } catch (e) { 605 if (e instanceof BusinessError) { 606 // Legacy does not throw on FindDocuments errors — it re …(truncated, 790 more chars)
OK, so the search method in DmpApi can still throw in other cases (e.g., session errors, network errors). The retry logic still makes sense for those cases. Network errors (non-BusinessError) still propagate through.
Now let me look at the useSearchDocuments retry behavior more carefully:
The retry callback only fires when queryFn throws/rejects. In the current design:
- Adapter returns { status: 'error', code: 'DMPAccessDeniedByExcededThreshold' } → queryFn returns data → NO retry (correct!)
- DmpApi throws a non-BusinessError → queryFn rejects → retry with failureCount check
- Network errors → queryFn rejects → 3 retries
Wait, but the retry for DMPAccessDeniedByExcededThreshold looking at error.message — is this now unreachable code? Since the adapter no longer throws for DMP API errors but returns them as data, when would a thrown error contain "DMPAccessDeniedByExcededThreshold" in its message?
Let me check if there are other layers that might throw with this message.
Tools used:
Bash:grep -rn "DMPAccessDeniedByExcededThreshold" /Users/USER/workspace/doctolib/doctolib/packages/ 2>/dev/null | head -20
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts:170: apiErrorCode: 'DMPAccessDeniedByExcededThreshold',
/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts:184: code: 'DMPAccessDeniedByExcededThreshold',
/Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts:46:
…(truncated, 1002 more chars)
Now checking the retry types. React Query v5's retry callback type signature — let me verify. The error parameter in retry is typed as Error (default). Since it's now returning from the search and no longer throwing, error?.message?.includes(...) is effectively a defensive leftover but still works.
Let me also verify the file path for DocumentLoincCodesToKind.ts that had "M" status in the initial git status — but that's unrelated to this PR:
Tools used:
Bash:git diff master...HEAD -- packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts | head -80
Tool results:
→ result (ok): diff --git a/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts b/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts index c618b88b07f9..df34ac38f61b 100644 --- a/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts +++ b/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts @@ -1,20 +1,24 @@ import { getPatientInsForDMP } from '@doctolib/dmp' -import type { …(truncated, 2922 more chars)
Tools used:
Bash:git diff master...HEAD -- packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.test.ts | head -140
Tool results:
→ result (ok): diff --git a/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.test.ts b/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.test.ts index f22a0aee4b1f..9740bd13d9a3 100644 --- a/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.test.ts +++ b/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.test.ts @@ -56,7 +56,7 @@ describe('useSearchDocuments', () => { // What we can verify at this layer: repeated calls …(truncated, 5424 more chars)
Now I have enough to write a thorough review. Let me finalize by checking one potential concern: the retry function receives a typed error. React Query v5's retry callback signature — let me verify the type is compatible:
Tools used:
Bash:grep -n "retry\?:\|retry:" /Users/USER/workspace/doctolib/doctolib/node_modules/@tanstack/react-query/build/legacy/hydration-5WZwLm1l.d.ts 2>/dev/null | head -5
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:find /Users/USER/workspace/doctolib/doctolib/node_modules/@tanstack/react-query -name "*.d.ts" 2>/dev/null | xargs grep -l "RetryValue\|RetryDelayValue" 2>/dev/null | head -3
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
Bash:find /Users/USER/workspace/doctolib/doctolib/node_modules/@tanstack/ -name "retryer.d.ts" 2>/dev/null | head -3
Tool results:
→ result (ok): /Users/USER/workspace/doctolib/doctolib/node_modules/@tanstack/query-core-v5/build/legacy/retryer.d.ts /Users/USER/workspace/doctolib/doctolib/node_modules/@tanstack/query-core-v5/build/modern/retryer.d.ts /Users/USER/workspace/doctolib/doctolib/node_modules/@tanstack/query-core/build/lib/retryer.d.ts
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib/node_modules/@tanstack/query-core-v5/build/modern/retryer.d.ts
Tool results:
→ result (ok): 1 export { C as CancelledError, br as RetryDelayValue, bq as RetryValue, bp as Retryer, bs as canFetch, bt as createRetryer, l as isCancelledError } from './hydration-DtrabBHC.js'; 2 import './removable.js'; 3 import './subscribable.js'; 4
Tools used:
Bash:grep -A 2 "type RetryValue\|RetryValue<\|type ShouldRetryFunction\|type RetryFn" /Users/USER/workspace/doctolib/doctolib/node_modules/@tanstack/query-core-v5/build/modern/hydration-DtrabBHC.d.ts …(truncated, 22 more chars)
Tool results:
→ result (ok): retry?: RetryValue<TError>; retryDelay?: RetryDelayValue<TError>; networkMode: NetworkMode | undefined; -- type RetryValue<TError> = boolean | number | ShouldRetryFunction<TError>; type ShouldRetryFunction<TError = DefaultError> = (failureCount: number, error: TError) => boolean; type RetryDelayValue<TError> = number | RetryDelayFunction<TError>; type RetryDelayFunction<TError = DefaultError> = (failureCount: number, error: TError) => number; -- retry?: RetryValue<TError>; retryD …(truncated, 4300 more chars)
Good, the error in retry callback is typed as the generic TError (defaults to DefaultError = Error). The error?.message?.includes(...) uses optional chaining which is defensive and works.
Now I have all the context I need. Let me present my findings.
Code Review
Files reviewed:
- /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts
- /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts
- /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.ts
- /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/hooks/useSearchDocuments.test.ts
- /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/src/variants/types.ts
- /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/national-health-records/package.json
- /Users/USER/workspace/doctolib/doctolib/packages/@doctolib/shared_patient_file_and_appointment_page_components/package.json
- /Users/USER/workspace/doctolib/doctolib/package.json and package-lock.json
Verification Summary
-
Error-as-data propagation pattern (correctness): The flow from
DmpApi→DmpNhrAdapter.searchDocuments→useSearchDocuments→SearchDocumentsResultis coherent.DmpApipopulatesSearchResult.errorwhens_status !== STATUS_OK; the adapter detects that, constructs the user-facing message throughnew DmpApiError(apiResponse.error).message, and preserves the stableapiErrorCodeincode. The hook branches onresult.status === 'error'and forwards bothmessageandcode(plus the network-error fallback in the catch block). -
TypeScript types:
NhrDocumentSearchResult = NhrResult & { documents: NhrDocument[] }acceptsstatus: 'success' | 'error' | 'skipped' | 'partial success'plus optionalmessage/code, so both returned shapes typecheck. ThequeryFnreturnsNhrDocumentSearchResult | nullvia the?? null— required because React Query v5 forbidsundefinedas query data.SearchDocumentsResultis now centrally defined intpf-national-health-records-uiand re-exported locally, aligning producer and consumer types. -
Retry logic (three scenarios): - (a) Adapter-returned errors: resolve through
fetchQuery, soretryis never evaluated — correct, no retry. - (b) Thrown errors whose message includesDMPAccessDeniedByExcededThreshold:retryreturnsfalse— no retry. - (c) Other thrown errors (network, session missing, etc.): retried up toDMP_DOCUMENTS_MAX_RETRIES = 3— correct. -
Tests: The new test
"returns error result when adapter returns status error"asserts bothmessageandcodeare propagated and verifies no retry occurs (toHaveBeenCalledTimes(1)). TheDmpNhrAdapter.test.tstest forapiResponse.errorasserts the exact assembled message'Access denied: Quota exceeded', provingDmpApiErrormessage construction works end-to-end. Use ofjest.requireActualis appropriate to avoid redefiningDmpApiError. -
Code style: Adheres to TS rules in CLAUDE.md — functional components/hooks style maintained, no
any, no ESLint suppressions, kebab-case/PascalCase conventions observed.
Findings
No high-confidence issues (≥ 80) identified.
Lower-confidence observations (below threshold, for author awareness only — not blocking)
useSearchDocuments.ts:46— theretrycallback'serror?.message?.includes('DMPAccessDeniedByExcededThreshold')branch is now less load-bearing than before because theDMPAccessDeniedByExcededThresholderror is returned as data rather than thrown through the adapter's search path. It still guards against hypothetical thrown errors from deeper inDmpApi.search(e.g., a non-BusinessErrorthrown abovetoSearchResult), so it is defensible to leave in place. Confidence this is a real issue: ~35 (not reported).useSearchDocuments.ts:63— when the caught error is not anErrorinstance,messageis set toundefinedwhilestatus: 'error'.SearchDocumentsResult.messageis optional, so this is type-safe, but the UI will render an error state with no message. Confidence: ~40 (not reported).DmpNhrAdapter.test.ts— consider adding a test for an API error where onlyapiErrorDescription(noapiErrorExtendedInformations) is present to cover the message-assembly fallback path inDmpApiError; this overlap is already covered inDmpApiError's own unit tests per the recent refactor commit, so it's optional. Confidence: ~30 (not reported).
Conclusion
The changes correctly implement the error-as-data pattern through the DMP stack. Types are sound, retry semantics match the stated behaviours for all three error categories, and test coverage faithfully reflects the new contract. The code meets project standards — no blocking issues.