-
Notifications
You must be signed in to change notification settings - Fork 56
feat(scan): serve socket scan view from cached results with 202 polling (SURF-742)
#1425
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
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 |
|---|---|---|
| @@ -1,47 +1,78 @@ | ||
| import { debugDir, debugFn } from '@socketsecurity/registry/lib/debug' | ||
|
|
||
| import { queryApiSafeText } from '../../utils/api.mts' | ||
| import { queryApiSafeTextWithStatus } from '../../utils/api.mts' | ||
|
|
||
| import type { CResult } from '../../types.mts' | ||
| import type { SocketArtifact } from '../../utils/alert/artifact.mts' | ||
|
|
||
| // HTTP 202 Accepted: cached results are still being computed; poll again. | ||
| const HTTP_STATUS_ACCEPTED = 202 | ||
|
|
||
| export const CACHED_POLL_INITIAL_DELAY_MS = 1000 | ||
| export const CACHED_POLL_MAX_DELAY_MS = 10_000 | ||
| export const CACHED_POLL_TIMEOUT_MS = 10 * 60 * 1000 | ||
|
|
||
| function sleep(ms: number): Promise<void> { | ||
| return new Promise(resolve => { | ||
| setTimeout(resolve, ms) | ||
| }) | ||
| } | ||
|
|
||
| export async function fetchScan( | ||
| orgSlug: string, | ||
| scanId: string, | ||
| ): Promise<CResult<SocketArtifact[]>> { | ||
| const result = await queryApiSafeText( | ||
| `orgs/${orgSlug}/full-scans/${encodeURIComponent(scanId)}`, | ||
| 'a scan', | ||
| ) | ||
|
|
||
| if (!result.ok) { | ||
| return result | ||
| // Serve pre-computed results from the immutable store (`?cached=true`): a | ||
| // 200 carries the ndjson body, a 202 means the server enqueued a background | ||
| // job to compute them — poll with backoff until the results are ready, so | ||
| // callers only ever observe the final scan. | ||
| const path = `orgs/${orgSlug}/full-scans/${encodeURIComponent(scanId)}?cached=true` | ||
| const deadline = Date.now() + CACHED_POLL_TIMEOUT_MS | ||
| let delayMs = CACHED_POLL_INITIAL_DELAY_MS | ||
|
Member
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. Should double check this against what the actual full scan timeout is in depscan.
Collaborator
Author
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. Checked server-side: there is no fixed completion deadline to align with — the background compute is retried with backoff and its runtime scales with scan size, so any fixed client-side deadline is a heuristic. The SDK path that supersedes this PR makes the deadline explicit and configurable: socket-sdk-js's |
||
| for (;;) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| const result = await queryApiSafeTextWithStatus(path, 'a scan') | ||
| if (!result.ok) { | ||
| return result | ||
| } | ||
| if (result.data.status !== HTTP_STATUS_ACCEPTED) { | ||
| return parseArtifactsNdjson(result.data.text) | ||
| } | ||
| if (Date.now() >= deadline) { | ||
| return { | ||
| ok: false, | ||
| message: 'Scan results not ready', | ||
| cause: `The Socket API is still computing cached results for scan ${scanId} after ${CACHED_POLL_TIMEOUT_MS / 60_000} minutes (path: ${path}). Retry in a few minutes — the server keeps computing in the background.`, | ||
| } | ||
| } | ||
| // eslint-disable-next-line no-await-in-loop | ||
| await sleep(delayMs) | ||
| delayMs = Math.min(delayMs * 2, CACHED_POLL_MAX_DELAY_MS) | ||
| } | ||
| } | ||
|
|
||
| const jsonsString = result.data | ||
|
|
||
| // This is nd-json; each line is a json object | ||
| export function parseArtifactsNdjson( | ||
| jsonsString: string, | ||
| ): CResult<SocketArtifact[]> { | ||
| // This is nd-json; each line is a json object. | ||
| const lines = jsonsString.split('\n').filter(Boolean) | ||
| let ok = true | ||
| const data = lines.map(line => { | ||
| const data: SocketArtifact[] = [] | ||
|
|
||
| for (let i = 0, { length } = lines; i < length; i += 1) { | ||
| const line = lines[i]! | ||
| try { | ||
| return JSON.parse(line) | ||
| data.push(JSON.parse(line)) | ||
| } catch (e) { | ||
| ok = false | ||
| debugFn('error', 'Failed to parse scan result line as JSON') | ||
| debugDir('error', { error: e, line }) | ||
| return undefined | ||
| return { | ||
| ok: false, | ||
| message: 'Invalid Socket API response', | ||
| cause: | ||
| 'The Socket API responded with at least one line that was not valid JSON. Please report if this persists.', | ||
| } | ||
| } | ||
| }) as unknown as SocketArtifact[] | ||
|
|
||
| if (ok) { | ||
| return { ok: true, data } | ||
| } | ||
|
|
||
| return { | ||
| ok: false, | ||
| message: 'Invalid Socket API response', | ||
| cause: | ||
| 'The Socket API responded with at least one line that was not valid JSON. Please report if this persists.', | ||
| } | ||
| return { ok: true, data } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| process.env['SOCKET_CLI_API_TOKEN'] = 'test-token' | ||
| process.env['SOCKET_CLI_API_BASE_URL'] = 'https://api.socket.dev/v0/' | ||
|
|
||
| import nock from 'nock' | ||
| import { afterEach, beforeEach, describe, expect, it } from 'vitest' | ||
|
|
||
| import { fetchScan, parseArtifactsNdjson } from './fetch-scan.mts' | ||
|
|
||
| // Drives the real direct-API path through nock (an external HTTP double) rather | ||
| // than stubbing owned modules. cached scan reads hit ?cached=true and poll on | ||
| // 202 until a 200 arrives. | ||
| const BASE_HOST = 'https://api.socket.dev' | ||
|
|
||
| const NDJSON = | ||
| '{"type":"npm","name":"lodash","version":"4.17.21"}\n' + | ||
| '{"type":"npm","name":"react","version":"18.2.0"}\n' | ||
|
|
||
| describe('fetchScan', () => { | ||
| beforeEach(() => { | ||
| nock.cleanAll() | ||
| nock.disableNetConnect() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| nock.cleanAll() | ||
| nock.enableNetConnect() | ||
| }) | ||
|
|
||
| it('returns cached artifacts on a 200 cache hit', async () => { | ||
| nock(BASE_HOST) | ||
| .get('/v0/orgs/test-org/full-scans/scan-1') | ||
| .query({ cached: 'true' }) | ||
| .reply(200, NDJSON) | ||
|
|
||
| const result = await fetchScan('test-org', 'scan-1') | ||
|
|
||
| expect(result.ok).toBe(true) | ||
| expect((result as { data: unknown[] }).data).toEqual([ | ||
| { type: 'npm', name: 'lodash', version: '4.17.21' }, | ||
| { type: 'npm', name: 'react', version: '18.2.0' }, | ||
| ]) | ||
| }) | ||
|
|
||
| it('polls on 202 until the cached result is ready', async () => { | ||
| nock(BASE_HOST) | ||
| .get('/v0/orgs/test-org/full-scans/scan-2') | ||
| .query({ cached: 'true' }) | ||
| .reply(202, { status: 'processing', id: 'scan-2' }) | ||
| nock(BASE_HOST) | ||
| .get('/v0/orgs/test-org/full-scans/scan-2') | ||
| .query({ cached: 'true' }) | ||
| .reply(200, NDJSON) | ||
|
|
||
| const result = await fetchScan('test-org', 'scan-2') | ||
|
|
||
| expect(result.ok).toBe(true) | ||
| expect((result as { data: unknown[] }).data).toHaveLength(2) | ||
| }) | ||
|
|
||
| it('maps a 404 to a failed CResult', async () => { | ||
| nock(BASE_HOST) | ||
| .get('/v0/orgs/test-org/full-scans/missing') | ||
| .query({ cached: 'true' }) | ||
| .reply(404, { error: { message: 'Not found' } }) | ||
|
|
||
| const result = await fetchScan('test-org', 'missing') | ||
|
|
||
| expect(result.ok).toBe(false) | ||
| expect(result).toMatchObject({ | ||
| message: 'Socket API error', | ||
| data: { code: 404 }, | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('parseArtifactsNdjson', () => { | ||
| it('parses one artifact per line, skipping blanks', () => { | ||
| const result = parseArtifactsNdjson(NDJSON) | ||
| expect(result).toEqual({ | ||
| ok: true, | ||
| data: [ | ||
| { type: 'npm', name: 'lodash', version: '4.17.21' }, | ||
| { type: 'npm', name: 'react', version: '18.2.0' }, | ||
| ], | ||
| }) | ||
| }) | ||
|
|
||
| it('returns an error when a line is not valid JSON', () => { | ||
| const result = parseArtifactsNdjson('{"ok":true}\nnot-json\n') | ||
| expect(result.ok).toBe(false) | ||
| expect(result).toMatchObject({ message: 'Invalid Socket API response' }) | ||
| }) | ||
| }) |
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.
Any reason this isn't implemented in the SDK and then just consumed in the CLI? Clients of that shouldn't need to implement this logic.
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.
No reason, and that is where it ended up. This PR is withdrawn in favor of the SDK owning the behavior:
socket-sdk-jsnow ships the cached-scan poll helper (src/utils/poll.mts), so every client gets the 202 handling for free, and the CLI consumes it through the sdk 4.x bump in #1427.