Skip to content

fix(api): answer 200 when a key-value store record is overwritten - #116

Closed
Pijukatel wants to merge 1 commit into
claude/review-referencesfrom
fix/record-upsert-status
Closed

Pijukatel wants to merge 1 commit into
claude/review-referencesfrom
fix/record-upsert-status

Conversation

@Pijukatel

@Pijukatel Pijukatel commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Storing a record answered 201 whether the key was new or already there. An overwrite now answers 200, so a client can tell a first write from an update without a second request.

Storing a record answered 201 whether the key was new or not. An overwrite now
answers 200, so a client can tell a first write from an update.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017ocuAL43YR6VPaBRrRfBbL
@apify-service-account

apify-service-account commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Platform parity:

  • PUT/POST .../records/:recordKey answers 200 when the key already exists (201 only for a new key) vs the platform always answering 201, overwrite or not, for both methods; not in requirements — runtime · platform · PUT/POST share the handler

Reviewed commit: f6ac1f2

@apify-factory apify-factory left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to fix before merge.

await store.setValue(recordKey, body, { contentType });
await touchStorage(record.id);
res.status(201).end();
res.status(existed ? 200 : 201).end();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important: the real platform answers 201 to every record store, overwrite or not (apify-core src/api/src/routes/key_value_stores/record.ts:168, res.status(201).json({})), and requirements/ documents no difference, so an overwrite answering 200 makes the emulator diverge from the platform in a way clients can observe.

@apify-factory apify-factory left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to fix before merge.

Outside the diff:

  • src/api/routes/key-value-stores.ts: Important: the real platform answers 201 for every record write, new key or overwrite (apify-core src/api/src/routes/key_value_stores/record.ts:168), and requirements/storage.md's "Known differences" does not list this, so an overwrite answering 200 here is an undocumented divergence that code relying on the platform's behavior will observe.

@Pijukatel
Pijukatel changed the base branch from claude/ecstatic-euler-ki5zev to claude/review-references November 3, 2026 10:38
@Pijukatel

Copy link
Copy Markdown
Collaborator Author

Comparing platform parity review with enhanced context apify_factory review. Seems to be working fine

@Pijukatel Pijukatel closed this Nov 3, 2026
@Pijukatel
Pijukatel deleted the fix/record-upsert-status branch November 3, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants