Attempting to update a document with revision comparison in 7.2.0 causes conflict

[robross0606]

This unit test fails:

  it.only('Document updates with revision checking causes conflict', async () => {
    const testCount = 10

    const db = arangoService.getInstance()
    const collection = await db.createCollection('test')
    const collectionData = await collection.get()
    expect(collectionData.type).toBe(CollectionType.DOCUMENT_COLLECTION)

    // Create documents
    const created = []
    for (let i = 0; i < testCount; i++) {
      created.push(await collection.save({ test: 'update-failure', count: 1 }, { returnNew: true }))
    }
    expect(created).toBeArrayOfObjects()
    expect(created).toBeArrayOfSize(testCount)

    // Now try to update them
    const updated = []
    for (const doc of created) {
      updated.push(
        await collection.update(
          doc._id,
          { _rev: doc._rev, updated: true },
          { returnOld: true, returnNew: true, ignoreRevs: false }
        )
      )
    }
    expect(updated).toBeArrayOfObjects()
    expect(updated).toBeArrayOfSize(testCount)
  })

The error is:

    ArangoError: conflict

      at new ArangoError (node_modules/src/error.ts:143:17)
      at Object.resolve (node_modules/src/connection.ts:900:20)
      at callback (node_modules/src/connection.ts:628:16)

If ignoreRevs is set to true or the _rev property is not provided on the document content, the test passes.

[robross0606]

I have also tried with a single document update which also fails with the same error:

  it.only('Document update with revision checking causes conflict', async () => {
    const db = arangoService.getInstance()
    const collection = await db.createCollection('test')
    const collectionData = await collection.get()
    expect(collectionData.type).toBe(CollectionType.DOCUMENT_COLLECTION)

    // Create document
    const created = await collection.save({ test: 'update-failure' }, { returnNew: true })
    expect(created).toBeNonEmptyObject()
    expect(created._id).toBeNonEmptyString()
    expect(created._rev).toBeNonEmptyString()
    expect(created.new.test).toBe('update-failure')
    expect(created.new.updated).toBeFalsy()

    // Now try to update
    const updated = await collection.update(
      created._id,
      { _rev: created._rev, updated: true },
      { returnOld: true, returnNew: true, ignoreRevs: false }
    )
    expect(updated).toBeNonEmptyObject()
    expect(updated._id).toBe(created._id)
    expect(updated._rev).not.toBe(created._rev)
    expect(updated.new.test).toBe(created.new.test)
    expect(updated.new.updated).toBeTruthy()
  })

Failure:

    ArangoError: conflict

      at new ArangoError (node_modules/src/error.ts:143:17)
      at Object.resolve (node_modules/src/connection.ts:900:20)
      at callback (node_modules/src/connection.ts:628:16)
      at IncomingMessage.<anonymous> (node_modules/src/lib/request.node.ts:165:15)

[robross0606]

With the old driver where rev was passed in as an option, this resulted in an HTTP API header value:

if-match: _bkexiY----

However, with the new API, there is no if-match header value. The value is passed in as a _rev in the body and this fails for some reason even though the HTTP API docs read for all the world like this should succeed:

If the If-Match header is specified and the revision of the document in the database is unequal to the given revision, the precondition is violated.

If If-Match is not given and ignoreRevs is false and there is a _rev attribute in the body and its value does not match the revision of the document in the database, the precondition is violated.

I have attached two Wireshark PCAP captures showing the difference between the two drivers using effectively the same unit test.

PCAP.zip

[robross0606]

I think the documentation on HTTP API is incorrect, or at least misleading. I tried this directly from the REST "Try It" feature on the Support page of the web UI and it fails in a similar manner. If using the If-Match header it works fine. If using a _rev stored in the body of the input, it fails:

[pluma]

Looks like the HTTP API docs are incorrect or misleading. It's probably easiest to restore the if-match behavior of v6, though I think the new option should be called ifMatch rather than rev.

[robross0606]

My only issue with calling it ifMatch is you're intentionally breaking backward compatibility with previous versions of your library. Putting back rev would make it backward compatible. This isn't a new option. It is an old option that was removed. Either way, I'll take it if it works. :)

[robross0606]

FYI, underlying Arango bug (arangodb/arangodb#13232) is confirmed.

[Simran-B]

The server-side issue is fixed in 3.6.12 and 3.7.7, but it may still be desirable to have a driver-side workaround for earlier versions.

[robross0606]

@Simran-B, trying to plan for our own projects. Any idea if/when a "driver-side workaround for earlier versions" might be implemented?

[robross0606]

@Simran-B, I'm trying to figure out if this change now makes the ArangoJS 7.x driver incompatible with older servers. This would appear to be the case as attempting to put _rev in the input with ignoreRevs: false works okay with 3.7.9 but breaks against a 3.6.5 server with a " precondition failed" error. Is there some way to use the 7.x driver update() call with revision checks that is compatible with both 3.6.x and 3.7.x servers?

[robross0606]

@Simran-B, looks like this only fails on servers earlier than 3.6.12 so that may well be what you mean by:

The server-side issue is fixed in 3.6.12 and 3.7.7, but it may still be desirable to have a driver-side workaround for earlier versions.

I wasn't expecting it to fail outright on earlier versions now.

[robross0606]

@Simran-B here are some wireshark captures of the update HTTP PATCH calls to compare responses from a 3.7.9, 3.6.12 and 3.6.5 server. 3.7.9 success.pcapng.log 3.6.12 success.pcapng.log 3.6.5 failure.pcapng.log The driver is clearly doing the same thing every time. But the earlier versions of the server do not like the changes.

[robross0606]

@Simran-B, just noticed workaround hinted at in https://github.com/arangodb/arangodb/issues/13232#issuecomment-764728178. Adding the _key to the input does indeed appear to make calls cross-compatible with old and new versions.