n1qlTimeout exceeded without errors and partial data is received

Hi,
It seems that if n1qlTimeout is exceeded then there is no error but Instead the result is returned as success but with partial data.
It means that when getting back a response from N!QL, there is no way to know if the data is full (i.e no timeout) or that there was a timeout and now the data is partial.
Am I missing something? What is the expected behaviour ?

Hi @aterem,
I am not too expert on the timeouts on client side, but I am guessing n1ql_timeout is passed straight to the n1ql service, rather that being handled on the client side like operation_timeout.

If this is the case, yes the n1ql service will apply the timeout in flight and terminate the request with partial results.

As part of that, it will return in the metrics section a status of “timeout” as opposed to “success”, which is the state for requests that complete normally.

There are other interesting statuses (“stopped”, “terminated”), but they are not relevant for the purpose of this discussion.
I digress! N1ql will also return error 4080 (timeout exceeded).
You don’t say what SDK you are using - but if in your callback function, upon seeing the final response flag, you should be able to determine how the request terminated.

@ingenthr might be able to shed some more light.

@aterem would it be possible to provide some more code/insight so we can try to reproduce your issue?

Thanks @marcog for replaying. I’m using nodejs and calling query as follow (for the sake of argument):

const query = N1qlQuery.fromString('SELECT vodContentMetaData.* FROM vodContentMetaData');
try {
    bucket.query(query, function (err, resp) {
        console.info(err, resp)
    });
}
catch(e){
    return cb(e);
}

I expected that timeout would be considered as error - hence the err parameter wouldn’t be null.
Looking at the sample in https://developer.couchbase.com/documentation/server/4.0/sdks/node-2.0/n1ql-queries.html I see that the error handling is done purly on the err variable and not on the meta variable. Also in the meta variable (I tried it only following your response) timeout is considered error (errorCount: 1) So I think there is some inconsistency here.

@aterem - waiting to hear from somebody in the know!
Will keep you posted…

My expectation is that if it times out at either N1QL or the client, you would receive an error. I’m not sure what you mean on “on the error variable”. Are you referring to the fact that some N1QL responses may be HTTP 200s with errors embedded?

@brett19 may have a thought.

Hi @ingenthr.
Just to clarify, following @marcog answer to pick in the meta variables for extra errors, this issue is now “workaroundable” for me.
And it can be easily, properly, fixed in the coucbnode SDK, here is a PR to demonstrate a potential fix:

To your questions:

I’m not sure what you mean on “on the error variable”

In node.js, it is considered standard practice to handle errors in asynchronous functions by returning them as the first argument to the current function’s callback. If there is an error, the first parameter is passed an Error object with all the details. Otherwise, the first parameter is null.
(https://docs.nodejitsu.com/articles/errors/what-are-the-error-conventions/)

An example can be found in couchbase’s samples here: (do search for err,)

Are you referring to the fact that some N1QL responses may be HTTP 200s with errors embedded

From user POV, I’m not making an HTTP call, I’m making a query (bucket.query) and I do not receive any http responses, what I’m getting is an error, results in the callback parameters. From user POV if error is null then the query finished with success - that is the nodejs standard…

Thanks,
Amotz

Hey @aterem,

I just did some testing locally and am able to reproduce the behaviour you are seeing. If the timeout is low enough that no results are returned, we get the expected result of seeing an error in Node.js. If however, there is a number of rows returned, and then midway through the rows it times out, the behaviour changes and we see a successful query but only a partial list of rows.

I am looking into solutions, and they will definitely be in the next release, you can track the issue here: https://issues.couchbase.com/browse/JSCBC-377. Note that while your solution does work, it doesn’t allow you to access information on why the query failed, which we would like to include. Hopefully we can find a solution that includes both the error that occurred, as well as the rows.

Cheers, Brett

Hi @brett19.
Did you try to run with my solution? You should be able to see the actual error. I’m testing the error count but returning the error array. Try to print err in the callback. You should see something like that
[ { code: 4080, msg: 'Timeout 2s exceeded' } ]

Thanks,
Amotz

Hey @aterem,

I’ve located the source of the problem, and uploaded one possible solution here: http://review.couchbase.org/#/c/78928/1. This change causes all secondary errors (errors occuring after rows have been received) to be reported through the callback (previously they were only available through the callbacks 3rd parameter, meta.errors). Do you think that this kind of behaviour change would be acceptable?

Cheers, Brett

Hi @brett19.
I think that the behaviour of reporting the error through the first parameter rather than hidden in the 3rd is correct behavior.

BTW Looking at the code review, I see the following issues

  1. Seems that you’ve deleted the errors object from meta ( 682: delete meta.errors;) I don’t think this is a good idea, because you don’t know who (today) is already relying on this parameter, I would keep it for backward compatibility (also it is actually not a mistake…)

  2. Wrong parameter passing, second parameter in error emit, is meta in one place and jsonError in the second
    emitter.emit(‘error’, err, meta, rows);
    emitter.emit(‘error’, err, jsonError);
    (Will result in inconsistent arguments to caller)

  3. Not declaring of the err variable (will result in it becoming global)

  4. I wonder if we can avoid the duplication of the code, seems like the new changes are word to word from the else scope.

Just in case other developers get here:

I’ve seen also other cases (not timeout) where there is an error in the server that is not reflected in the err object. In these cases I just got an empty array without any errors

The way around it is to also add a check of meta.metrics.errorCount to know if there was an error in the server that was not reflected in err object

bucket.query(query,  (err,res,meta)  {
                if (err || meta.metrics.errorCount > 0 ) {
                    
                } 
            });