Skip to content

fix: handle precondition-failed error type in document PUT/POST responses - #106

Merged
milt merged 1 commit into
yetanalytics:masterfrom
pelotech:fix/atomic-etag-validation
Aug 10, 2026
Merged

fix: handle precondition-failed error type in document PUT/POST responses#106
milt merged 1 commit into
yetanalytics:masterfrom
pelotech:fix/atomic-etag-validation

Conversation

@chomatdam

@chomatdam chomatdam commented Feb 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • put-response and post-response in the document routes now handle :com.yetanalytics.lrs.xapi.document/precondition-failed errors by returning HTTP 412 (Precondition Failed)
  • This enables downstream implementations (lrsql) to return structured error maps from -set-document when If-Match / If-None-Match validation fails, and have them correctly surfaced as 412 responses

Context

The existing etag validation in etags-preproc runs in a separate interceptor stage before the actual document write. This creates a TOCTOU (Time-of-Check-Time-of-Use) race condition — between the etag check and the write, another request can modify the document, causing silent overwrites despite correct use of concurrency headers.

To fix this, the downstream LRS implementation needs to perform atomic check-then-write within a single DB transaction. This PR provides the error-handling plumbing so that a precondition failure detected at write time is correctly propagated as a 412.

Changes

  • put-response: Check ex-data for ::precondition-failed type → 412
  • post-response: Add ::precondition-failed → 412 branch (using cond instead of nested if for clarity alongside existing 400 error types)

Companion PR

Requires corresponding changes in lrsql (atomic etag validation in -set-document) to produce the :com.yetanalytics.lrs.xapi.document/precondition-failed error type:
yetanalytics/lrsql#504

Testing

Tested via lrsql make test-sqlite:

  • xAPI 1.0.3 conformance: 1365/1365 passed
  • xAPI 2.0.0 conformance: 1435/1435 passed
  • Unit tests: 88 tests, 923 assertions, 0 failures, 0 errors

@chomatdam chomatdam changed the title fix: handle precondition failed error in document PUT/POST responses fix: Handle precondition-failed error type in document PUT/POST responses Feb 15, 2026
@chomatdam chomatdam changed the title fix: Handle precondition-failed error type in document PUT/POST responses fix: handle precondition-failed error type in document PUT/POST responses Feb 16, 2026
@milt
milt self-requested a review August 10, 2026 15:18

@milt milt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a subset of the work @cliffcaseyyet mentions we need to do here yetanalytics/lrsql#504 (comment), the error name is sensible and the handling is correct so I'm going to approve + merge this and then add the rest of the applicable actions and routes. Thanks again @chomatdam !

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.

2 participants