SK-2771: fix deidentifyFile crash when waitTime is exceeded before processing completes#296
Conversation
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
There was a problem hiding this comment.
Pull request overview
Fixes a crash in deidentifyFile when waitTime expires before the detect run finishes by making the poller resolve shape consistent and preventing fall-through into response parsing on the timeout path.
Changes:
- Standardized poller resolution to
{ data, runId }for both timeout and success paths. - Updated
.then()handling to use named destructuring and added an earlyreturnforIN_PROGRESS. - Added unit tests covering the timeout /
IN_PROGRESSbehavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/vault/controller/detect/index.ts |
Unifies poll resolve shape and prevents parsing on timeout (IN_PROGRESS) responses. |
test/vault/controller/detect.test.js |
Adds tests asserting correct return shape and no parsed file data when waitTime is exceeded. |
Comments suppressed due to low confidence (1)
src/vault/controller/detect/index.ts:612
promiseReqis typed asDeidentifyFileDetectRunResponse & { runId?: string; status?: string }, but the timeout path resolvesdatawith only{ runId, status: 'IN_PROGRESS' }(missing requiredoutput,outputType,message, etc. fromDeidentifyFileDetectRunResponse). This makes the type misleading and can hide real runtime shape differences. Consider introducing a discriminated union for the poll result (e.g.,{ runId, data: DeidentifyFileDetectRunResponse } | { runId, data: { status: 'IN_PROGRESS' } }) and narrowing in the.thenhandler, or relaxingdatato aPartial<DeidentifyFileDetectRunResponse>with an explicit guard before callingparseDeidentifyFileResponse.
var reqType : DeidenitfyFileRequestTypes = this.getReqType(fileExtension);
var promiseReq: Promise<{ data: DeidentifyFileDetectRunResponse & { runId?: string; status?: string }, runId: string }>;
switch (reqType){
case DeidenitfyFileRequestTypes.AUDIO:
promiseReq = this.buildAudioRequest(fileObj, options, fileExtension)
.then((audioReq) => {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Problem
When waitTime is small and the file is still processing, the SDK crashed with TypeError: object is not iterable.
The poller resolved with { runId } (object) on timeout but [response, runId] (array) on success — the .then() handler always destructured as an array, causing the crash. A missing return also caused fall-through into response parsing with incomplete data.
Changes
Unified both resolve paths to consistent { data, runId } shape
Switched to named destructuring in .then() handler
Added return after IN_PROGRESS resolve to stop fall-through
Added unit tests for the timeout path
Verification
Tested locally with waitTime: 1 — previously crashed, now returns { runId, status: 'IN_PROGRESS' } cleanly.