Pass request options to Database.query() etc.

[Yogu]

I would like to have a way to pass some request-related options when calling high-level methods like Database.query(), Database.executeTransaction() etc. I'm mainly interested in beforeRequest and afterResponse, but I think all of CommonRequestOptions would make sense. In addition, the options could include a signal to be able to abort a request.

My use cases

• Our frontends cancel requests when they are no longer needed (e.g. when the user changes filter criteria). If this happens, I would like to abort the request to the ArangoDB server as well, or remove it from the task queue if it's not been started yet. This would be possible by throwing conditionally in beforeRequest, or with a new signal option that I could abort.
• I would like to track how long a request spent in the arangojs-internal request queue, and correlate this with contextual information. This is not possible with a beforeRequest callback per Database instance, but it would be possible with a beforeRequest option on the .query() call

Other potential use cases

• Different HTTP timeouts depending on the context (e.g. a longer timeout on queries you know will take a long time)
• Different values for retryOnConflict depending on the context, e.g. enable them for simple queries, but disable them if you want to retry on a higher level instead, or increase the number of retries for certain queries
• Set custom HTTP headers like a request ID that will be processed somewhere on the network stack, on a reverse proxy etc.

Suggested API

// New type. Not sure about the name - CommonRequestOptions would be fitting but is already taken for the global options
type ExtraRequestOptions = CommonRequestOptions & {
  signal: AbortSignal

  /**
   * Headers object containing any additional headers to send with the request.
   *
   * Note that the `Authorization` header will be overridden if the `auth`
   * configuration option is set.
   */
  headers?:
    | string[][]
    | Record<string, string | ReadonlyArray<string>>
    | Headers;
}

// Existing type
type QueryOptions = ExtraRequestOptions & {
  // ... existing properties
}

// Existing type
type TransactionOptions = ExtraRequestOptions & {  
  // ... existing properties
}

class Database {
  // Example for a method that currently does not have an options argument
  exists(options: ExtraRequestOptions) {
    // ...
  }
}

The new options could be merged into all option arguments that perform a request. Personally, I'm mostly interested in query, executeTransaction, beginTransaction, Transaction.commit(), Transaction.abort() and the cursor APIs (although we currently don't use cursors).

Alternatively, the request options could also be nested to separate them from the function-specific options

// Existing type
type QueryOptions = {
  // ... existing properties
  requestOptions: ExtraRequestOptions
}

[ghost]

Since the beforeRequest and afterResponse callbacks are passed the same request object (via the error or response object in the latter case), you could still identify the request you want to track this way, or am I seeing that wrong? I can see concurrency being an issue tho.

We'll likely not implement an API change like this as we already avoided this for other things that could potentially affect any request like streaming transactions.

[Yogu]

Since the beforeRequest and afterResponse callbacks are passed the same request object (via the error or response object in the latter case), you could still identify the request you want to track this way

That would allow to correlated beforeRequest to afterResponse. However, the mentioned use cases do not require correlating these two callbacks to each other, but to the call site.

Going through the "My use cases" (the "other potential use cases" are not related to beforeRequest/afterResponse)

Aborting an ArangoDB request when the user aborted their request

This means I need to throw in beforeRequest (to prevent the ArangoDB request from being sent). afterResponse is not involved at all. To know whether or not I need to throw in beforeRequest, I need context information from the place that called .query() etc., e.g. like this (simplified a lot):

app.get('/', (req, res) => {
  let aborted = false;
  res.once('close', () => { if (!res.writableFinished) { aborted = true; } }); // there should be an easier way but it's broken https://github.com/nodejs/node/issues/46666
  db.query(aql`FOR obj in collection return ...`, {
    beforeRequest: () => { if (aborted) { throw new Error('User aborted the request'); } }
  }).then(result => res.send(...));
});

I don't think there currently is a way to implement this without the feature I proposed here.

Time tracking

While you can implement a very basic time tracking using just beforeRequest and afterResponse, it would not be very useful without any kind of context information. For example, I would like to

• differentiate between queries and mutations and also other categories
• for slow queries, generate a log entry with the actual query string (or hash or id or anything that would let me find actual query string that was slow)
• find out how long a request spent in the arangojs-internal queue (call of .query() to beforeRequest)

All of this would be easy to implement if I could correlate the beforeRequest to the call site, but (as far as I understand) completely impossible without.

[ghost]

I think a way to implement this without having to add support to every single method would be similarly to how trx.step or the db.createJob helper works, i.e. by setting the before/after callbacks per-request using a helper method. The API could look something like this:

const queryResult = await db.withRequestOptions({
  beforeRequest: yourBeforeRequestHandler,
  afterResponse: yourAfterResponseHandler,
  signal: yourSignal
}, () => db.query(...));

The implementation could be done in db.request similarly to how db.createJob sets an internal _trapRequest callback, i.e. it would set an internal property like _nextRequestOptions that would then be applied (and unset) in db.request if present so it would only affect the next (synchronously invoked) request, i.e. the one in the callback provided to withRequestOptions. This could be used for overriding other request options per request too.

[pluma]

FYI @shd8 @cmyk47 this might also be a starting point for overriding auth per-request in Node Foxx if a request user's credentials should be used for calls to ArangoDB. Overriding the db object's credentials directly would affect ongoing parallel requests.

I'm no longer going to be with ArangoDB next month so it's on the others to actually implement this.