Repository navigation
fix(ingest): list per-event errors for rejected EPCIS batches #3578
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.
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
Changes from all commits
120f8b3
dd871b5
862188f
e89f3f0
10fd5ff
82323b8
626393c
6cb493d
c48f3e9
0f3dd09
093798b
0745207
89ea2b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { fireEvent, render, screen } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import IngestPage from '@/app/ingest/page'; | ||
|
|
||
| const mocks = vi.hoisted(() => ({ | ||
| ingestURL: vi.fn(), | ||
| ingestFile: vi.fn(), | ||
| ingestEpcisEvent: vi.fn(), | ||
| ingestEpcisBatch: vi.fn(), | ||
| ingestEpcisXml: vi.fn(), | ||
| validateEpcis: vi.fn(), | ||
| getIngestionJob: vi.fn(), | ||
| refresh: vi.fn(), | ||
| storeApiKey: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock('@/lib/auth-context', () => ({ | ||
| useAuth: () => ({ apiKey: 'cookie-managed', setApiKey: mocks.storeApiKey }), // pragma: allowlist secret | ||
| })); | ||
| vi.mock('@/hooks/use-api', () => ({ | ||
| useIngestURL: () => ({ mutateAsync: mocks.ingestURL, reset: vi.fn(), isPending: false, isError: false }), | ||
| useIngestFile: () => ({ mutateAsync: mocks.ingestFile, reset: vi.fn(), isPending: false, isError: false }), | ||
| useIngestEpcisEvent: () => ({ mutateAsync: mocks.ingestEpcisEvent, reset: vi.fn(), isPending: false }), | ||
| useIngestEpcisBatch: () => ({ mutateAsync: mocks.ingestEpcisBatch, reset: vi.fn(), isPending: false }), | ||
| useIngestEpcisXml: () => ({ mutateAsync: mocks.ingestEpcisXml, reset: vi.fn(), isPending: false }), | ||
| useValidateEpcisEvent: () => ({ mutateAsync: mocks.validateEpcis, reset: vi.fn(), isPending: false }), | ||
| })); | ||
| vi.mock('@/lib/api-client', () => ({ | ||
| apiClient: { getIngestionJob: mocks.getIngestionJob }, | ||
| })); | ||
| vi.mock('@/hooks/use-dashboard-refresh', () => ({ | ||
| notifyDashboardRefresh: mocks.refresh, | ||
| })); | ||
| vi.mock('@/app/ingest/NormalizedDocumentViewer', () => ({ | ||
| NormalizedDocumentViewer: () => <div>Normalized document viewer</div>, | ||
| })); | ||
|
|
||
| // Data-free placeholders: two {} events. Shapes below are copied from the | ||
| // backend source; counts derive from EVENTS.length. | ||
| const EVENTS: Record<string, unknown>[] = [{}, {}]; | ||
|
|
||
| // _validate_epcis (services/ingestion/app/epcis/validation.py) output for {}, | ||
| // in order. | ||
| const BACKEND_EVENT_ERRORS = [ | ||
| ...['type', 'eventTime', 'action', 'bizStep'].map((field) => `Missing required EPCIS field '${field}'`), | ||
| 'Unsupported EPCIS event type', | ||
| 'Missing traceability lot code (fsma:traceabilityLotCode or cbvmda:lotNumber)', | ||
| ]; | ||
| const STATUS = 400; | ||
| const JOINED = BACKEND_EVENT_ERRORS.join(': '); | ||
| // Exact message from _ingest_batch_events_db_atomic (persistence.py). | ||
| const ATOMIC_MESSAGE = | ||
| 'One or more events failed pre-DB validation. No events were persisted. ' | ||
| + 'Use ?mode=partial to opt in to per-event processing with HTTP 207 semantics.'; | ||
|
|
||
| function axiosRejection(status: number, data: unknown) { | ||
| return Object.assign(new Error(`Request failed with status code ${status}`), { response: { status, data } }); | ||
| } | ||
|
|
||
| // Atomic (default, _batch_transactional()): HTTPException(400, detail={...}). | ||
| function atomicRejection(events: unknown[]) { | ||
| return axiosRejection(STATUS, { | ||
| detail: { | ||
| error: 'batch_validation_failed', | ||
| mode: 'atomic', | ||
| message: ATOMIC_MESSAGE, | ||
| errors: events.map((_, index) => ({ index, status_code: STATUS, detail: { errors: BACKEND_EVENT_ERRORS } })), | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // Partial (?mode=partial or EPCIS_BATCH_TRANSACTIONAL=false): router.py | ||
| // returns the payload itself with status 400, no detail wrapper. | ||
| function partialRejection(events: unknown[]) { | ||
| return axiosRejection(STATUS, { | ||
| total: events.length, | ||
| created: 0, | ||
| failed: events.length, | ||
| results: [], | ||
| errors: events.map((_, index) => ({ index, detail: { errors: BACKEND_EVENT_ERRORS } })), | ||
| mode: 'partial', | ||
| }); | ||
| } | ||
|
|
||
| async function uploadBatch() { | ||
| render(<IngestPage />); | ||
| await userEvent.click(screen.getByRole('tab', { name: /epcis 2\.0/i })); | ||
| const input = document.getElementById('epcis-file-upload') as HTMLInputElement; | ||
| fireEvent.change(input, { | ||
| target: { files: [new File([JSON.stringify(EVENTS)], 'events.json', { type: 'application/json' })] }, | ||
| }); | ||
| await userEvent.click(screen.getByRole('button', { name: /upload & process/i })); | ||
| } | ||
|
|
||
| function expectEveryEventListed() { | ||
| for (let index = 0; index < EVENTS.length; index += 1) { | ||
| expect(screen.getByText(`Event ${index + 1}: ${JOINED}`)).toBeInTheDocument(); | ||
| } | ||
| expect(screen.getAllByText(/^Event \d+:/)).toHaveLength(EVENTS.length); | ||
| } | ||
|
|
||
| describe('EPCIS all-failed batch shows per-event errors', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('lists every event under the atomic message', async () => { | ||
| mocks.ingestEpcisBatch.mockRejectedValue(atomicRejection(EVENTS)); | ||
| await uploadBatch(); | ||
| expect(await screen.findByText('Ingestion Failed')).toBeInTheDocument(); | ||
| expect(screen.getByText(ATOMIC_MESSAGE)).toBeInTheDocument(); | ||
| expectEveryEventListed(); | ||
| expect(mocks.refresh).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('lists every event for a partial-mode 400 body', async () => { | ||
| const rejection = partialRejection(EVENTS); | ||
| mocks.ingestEpcisBatch.mockRejectedValue(rejection); | ||
| await uploadBatch(); | ||
| expect(await screen.findByText('Ingestion Failed')).toBeInTheDocument(); | ||
| expect(screen.getByText(rejection.message)).toBeInTheDocument(); | ||
| expectEveryEventListed(); | ||
| expect(mocks.refresh).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('shows the single-event 400 once, with no Event N: lines', async () => { | ||
| // Single-event path: persistence.py raises | ||
| // HTTPException(400, detail={"errors": [...]}) for {}. | ||
| mocks.ingestEpcisEvent.mockRejectedValue(axiosRejection(STATUS, { detail: { errors: BACKEND_EVENT_ERRORS } })); | ||
| render(<IngestPage />); | ||
| await userEvent.click(screen.getByRole('tab', { name: /epcis 2\.0/i })); | ||
| const input = document.getElementById('epcis-file-upload') as HTMLInputElement; | ||
| fireEvent.change(input, { | ||
| target: { files: [new File([JSON.stringify(EVENTS[0])], 'event.json', { type: 'application/json' })] }, | ||
| }); | ||
| await userEvent.click(screen.getByRole('button', { name: /upload & process/i })); | ||
| expect(await screen.findByText('Ingestion Failed')).toBeInTheDocument(); | ||
| expect(screen.getAllByText(JOINED)).toHaveLength(1); | ||
| expect(screen.queryAllByText(/^Event \d+:/)).toHaveLength(0); | ||
| expect(mocks.ingestEpcisBatch).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('clears the previous per-event errors when the API-key save fails', async () => { | ||
| mocks.ingestEpcisBatch.mockRejectedValue(atomicRejection(EVENTS)); | ||
| await uploadBatch(); | ||
| expect(await screen.findByText('Ingestion Failed')).toBeInTheDocument(); | ||
| expectEveryEventListed(); | ||
|
|
||
| mocks.storeApiKey.mockRejectedValue(new Error('API key could not be saved. Please try again.')); | ||
| fireEvent.change(screen.getByPlaceholderText('rge_...'), { target: { value: 'rge_replacement_key' } }); // pragma: allowlist secret | ||
| await userEvent.click(screen.getByRole('button', { name: /upload & process/i })); | ||
|
|
||
| expect(await screen.findByText('API key could not be saved. Please try again.')).toBeInTheDocument(); | ||
| expect(screen.queryAllByText(/^Event \d+:/)).toHaveLength(0); | ||
| expect(screen.queryByText(ATOMIC_MESSAGE)).not.toBeInTheDocument(); | ||
| expect(mocks.ingestEpcisBatch).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ import { | |
| parseEpcisJson, | ||
| normalizeEpcisIngestResult, | ||
| epcisErrorMessage, | ||
| epcisErrorEventMessages, | ||
| type ParsedEpcisJson, | ||
| } from '@/lib/epcis-upload'; | ||
| import { notifyDashboardRefresh } from '@/hooks/use-dashboard-refresh'; | ||
|
|
@@ -122,6 +123,9 @@ export default function IngestPage() { | |
| const [jobId, setJobId] = useState<string | null>(null); | ||
| const [jobStatus, setJobStatus] = useState<string>('queued'); | ||
| const [statusError, setStatusError] = useState<string | null>(null); | ||
| // URL-tab error for a failed API-key save; URL ingest errors otherwise come | ||
| // from ingestMutation, which has no setter. | ||
| const [urlError, setUrlError] = useState<string | null>(null); | ||
| // Dashboard "No data yet" tiles deep-link to /ingest?tab=file (CSV upload). | ||
| const [activeTab, setActiveTab] = useState<string>(() => { | ||
| const tab = searchParams.get('tab'); | ||
|
|
@@ -133,6 +137,7 @@ export default function IngestPage() { | |
| const [epcisFile, setEpcisFile] = useState<File | null>(null); | ||
| const [epcisResult, setEpcisResult] = useState<NormalizedEpcisResult | null>(null); | ||
| const [epcisError, setEpcisError] = useState<string | null>(null); | ||
| const [epcisErrorEvents, setEpcisErrorEvents] = useState<string[]>([]); | ||
| const [epcisValidation, setEpcisValidation] = useState<{ | ||
| checked: number; | ||
| valid: number; | ||
|
|
@@ -171,6 +176,7 @@ export default function IngestPage() { | |
| setEpcisFile(e.target.files[0]); | ||
| setEpcisResult(null); | ||
| setEpcisError(null); | ||
| setEpcisErrorEvents([]); | ||
| setEpcisValidation(null); | ||
| } | ||
| }; | ||
|
|
@@ -210,6 +216,26 @@ export default function IngestPage() { | |
| // browser proxy reads it from the cookie, then clear prior result state. | ||
| // Returns false when the caller should stop without sending any request. | ||
| const prepareSubmit = async (): Promise<boolean> => { | ||
| // Clear the previous upload's results and errors first, so a failed | ||
| // API-key save below never leaves stale error lines on screen. | ||
| setJobId(null); | ||
| setJobStatus('queued'); | ||
| setStatusError(null); | ||
| setCsvResult(null); | ||
| setCsvError(null); | ||
| setCteSuggestion(null); | ||
| setErrorColumnSuggestions([]); | ||
| setEpcisResult(null); | ||
| setEpcisError(null); | ||
| setEpcisErrorEvents([]); | ||
| setEpcisValidation(null); | ||
| setUrlError(null); | ||
| ingestMutation.reset(); | ||
| ingestFileMutation.reset(); | ||
| ingestEpcisEventMutation.reset(); | ||
| ingestEpcisBatchMutation.reset(); | ||
| ingestEpcisXmlMutation.reset(); | ||
| validateEpcisMutation.reset(); | ||
| // Store the API key for future use | ||
| if (!isAuthenticated && apiKey !== storedApiKey) { | ||
| // The browser proxy reads the cookie written by setApiKey and ignores | ||
|
|
@@ -221,20 +247,10 @@ export default function IngestPage() { | |
| const message = error instanceof Error ? error.message : 'Unable to save API key'; | ||
| if (activeTab === 'file') setCsvError(message); | ||
| else if (activeTab === 'epcis') setEpcisError(message); | ||
| else setStatusError(message); | ||
| else setUrlError(message); | ||
| return false; | ||
| } | ||
| } | ||
| setJobId(null); | ||
| setJobStatus('queued'); | ||
| setStatusError(null); | ||
| setCsvResult(null); | ||
| setCsvError(null); | ||
| setCteSuggestion(null); | ||
| setErrorColumnSuggestions([]); | ||
| setEpcisResult(null); | ||
| setEpcisError(null); | ||
| setEpcisValidation(null); | ||
| return true; | ||
| }; | ||
|
|
||
|
|
@@ -308,6 +324,7 @@ export default function IngestPage() { | |
| notifyDashboardRefresh(); | ||
| } catch (error) { | ||
| setEpcisError(epcisErrorMessage(error)); | ||
| setEpcisErrorEvents(epcisErrorEventMessages(error)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. URL error skips load-error classLow Severity The new Reviewed by Cursor Bugbot for commit 89ea2b5. Configure here. |
||
| } finally { | ||
| submitInFlightRef.current = false; | ||
| setIsSubmitting(false); | ||
|
|
@@ -818,14 +835,17 @@ export default function IngestPage() { | |
| </Callout> | ||
| )} | ||
|
|
||
| {((activeTab === 'url' && ingestMutation.isError) || (activeTab === 'file' && csvError) || (activeTab === 'epcis' && epcisError)) && ( | ||
| {((activeTab === 'url' && (urlError || ingestMutation.isError)) || (activeTab === 'file' && csvError) || (activeTab === 'epcis' && epcisError)) && ( | ||
| <Callout tone="danger" role="alert" className="mt-4"> | ||
| <h4 className="font-medium text-re-text-primary"> | ||
| Ingestion Failed | ||
| </h4> | ||
| <p className="mt-1"> | ||
| {(activeTab === 'file' ? csvError : activeTab === 'epcis' ? epcisError : ingestMutation.error?.message) || 'An error occurred during ingestion'} | ||
| {(activeTab === 'file' ? csvError : activeTab === 'epcis' ? epcisError : urlError || ingestMutation.error?.message) || 'An error occurred during ingestion'} | ||
| </p> | ||
| {activeTab === 'epcis' && epcisErrorEvents.map((message, index) => ( | ||
| <p key={index} className="mt-2">{message}</p> | ||
| ))} | ||
| {activeTab === 'file' && ( | ||
| <ColumnSuggestionList suggestions={errorColumnSuggestions} /> | ||
| )} | ||
|
|
@@ -878,7 +898,9 @@ export default function IngestPage() { | |
| setEpcisFile(null); | ||
| setEpcisResult(null); | ||
| setEpcisError(null); | ||
| setEpcisErrorEvents([]); | ||
| setEpcisValidation(null); | ||
| setUrlError(null); | ||
| setJobId(null); | ||
| setJobStatus('queued'); | ||
| ingestMutation.reset(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,27 @@ export function normalizeEpcisIngestResult(data: EpcisIngestResponse | null | un | |
| return { total, accepted, rejected, errorMessages }; | ||
| } | ||
|
|
||
| /** | ||
| * Per-event lines for a rejected batch, in both backend shapes: | ||
| * atomic (400, detail {error, mode, message, errors:[{index, status_code, detail}]}) | ||
| * and partial (400 body with no detail wrapper: {errors:[{index, detail}], ...}). | ||
| * Returns [] for any other failure. | ||
| */ | ||
| export function epcisErrorEventMessages(error: unknown): string[] { | ||
| if (!(error instanceof Error)) return []; | ||
| const data = (error as Error & { response?: { data?: unknown } }).response?.data; | ||
| if (!isRecord(data)) return []; | ||
| const source = isRecord(data.detail) ? data.detail : data; | ||
| if (!Array.isArray(source.errors)) return []; | ||
| const messages: string[] = []; | ||
| for (const entry of source.errors) { | ||
| if (!isRecord(entry) || typeof entry.index !== 'number') continue; | ||
| const message = resultItemMessage(entry as { index?: number; detail?: unknown }); | ||
| if (message && !messages.includes(message)) messages.push(message); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a For a large rejected batch, 🧰 Tools
|
||
| } | ||
| return messages; | ||
| } | ||
|
|
||
| /** Render a backend or network failure the same terse way the CSV tab does. */ | ||
| export function epcisErrorMessage(error: unknown): string { | ||
| if (error instanceof Error) { | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔎 Supported by static analysis
Script executed:
Repository: regengine/RegEngine
Length of output: 7332
Move the job-state reset until API-key storage succeeds.
prepareSubmitclearsjobIdbeforestoreApiKeyruns. If the save fails, the function returnsfalse, so no new request starts. The running job's polling stops, and its job badge and status are hidden. Keep the job state until the save succeeds, so a failed save leaves the active job visible.🐛 Suggested fix
const prepareSubmit = async (): Promise<boolean> => { // Clear the previous upload's results and errors first, so a failed // API-key save below never leaves stale error lines on screen. - setJobId(null); - setJobStatus('queued'); - setStatusError(null); setCsvResult(null);else setUrlError(message); return false; } } + setJobId(null); + setJobStatus('queued'); + setStatusError(null); return true; };