Skip to content

Reject non-orchestrator job ids from $import status endpoint - #5800

Merged
Richa Bansal (rbans96) merged 4 commits into
mainfrom
rbans96-reject-non-orchestrator-import-job-ids
Sep 11, 2026
Merged

Reject non-orchestrator job ids from $import status endpoint#5800
Richa Bansal (rbans96) merged 4 commits into
mainfrom
rbans96-reject-non-orchestrator-import-job-ids

Conversation

@rbans96

@rbans96 Richa Bansal (rbans96) commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Description

GET /_operations/import/{id} threw an unhandled ArgumentNullException (Parameter 'uriString') and returned HTTP 500 when status was requested using a processing job id instead of the orchestrator's.

The handler fetches the job by the requested id, then reads the rest of the group and excludes that job. When the requested id belongs to a processing job, the excluded record is that worker rather than the orchestrator, so the orchestrator stayed in the list and its definition was deserialized as an ImportProcessingJobDefinition. ImportOrchestratorJobDefinition has no resourceLocation property, so ResourceLocation was null and new Uri(null) threw.

The previous fix (#5748) tolerated the null location downstream, which stopped the 500 but not the absolute fix.

Adds a debugging rule to AGENTS.md covering invariant identification and entry-point validation when fixing unhandled exceptions.

Related issues

Addresses #204980

Testing

Describe how this change was tested.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

GET /_operations/import/{id} threw an unhandled ArgumentNullException
(Parameter 'uriString') and returned HTTP 500 when status was requested using a
processing job id instead of the orchestrator's.

The handler fetches the job by the requested id, then reads the rest of the
group and excludes that job. When the requested id belongs to a processing job,
the excluded record is that worker rather than the orchestrator, so the
orchestrator stayed in the list and its definition was deserialized as an
ImportProcessingJobDefinition. ImportOrchestratorJobDefinition has no
resourceLocation property, so ResourceLocation was null and new Uri(null) threw.

The previous fix (#5748) tolerated the null location downstream, which stopped
the 500 but returned a partial body built from the wrong job: a null Request, the
worker's create date as TransactionTime, and the polled worker missing from its
own output. This reverts that change and enforces the contract at the entry
point instead. A processing job id was never a valid import job id, so it is now
rejected with 404, matching the behavior of the bulk delete and bulk update
status handlers, which read the group directly and are structurally immune.

The queue assigns the orchestrator a job id equal to its group id, and processing
jobs are enqueued into that same group, so Id != GroupId identifies a processing
job. CreateImportRequestHandler only ever returns the orchestrator id to callers.

Adds a unit test asserting 404 for a processing job id, and an E2E test that runs
a real import, reads a processing job id from its group, and asserts the status
endpoint returns 404 for it.

Adds a debugging rule to AGENTS.md covering invariant identification and
entry-point validation when fixing unhandled exceptions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@rbans96
Richa Bansal (rbans96) requested a review from a team as a code owner September 10, 2026 19:37
@rbans96 Richa Bansal (rbans96) added Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs No-ADR ADR not needed Bug Bug bug bug. labels Sep 10, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…mport E2E test

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.85%. Comparing base (ef77cdc) to head (a3ce479).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5800      +/-   ##
==========================================
+ Coverage   78.29%   78.85%   +0.56%     
==========================================
  Files        1016     1016              
  Lines       36911    36901      -10     
  Branches     5617     5612       -5     
==========================================
+ Hits        28898    29098     +200     
+ Misses       6635     6419     -216     
- Partials     1378     1384       +6     

see 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rbans96
Richa Bansal (rbans96) merged commit e7ce65b into main Sep 11, 2026
48 of 50 checks passed
@rbans96
Richa Bansal (rbans96) deleted the rbans96-reject-non-orchestrator-import-job-ids branch September 11, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Bug Bug bug bug. No-ADR ADR not needed No-PaaS-breaking-change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants