test: reproduce commonly cited timeout error #1390
Conversation
- Added tests to ensure error is passed back to user - Changed source code to use passthrough for runQuery - changed the mock server to allow overriding of the default service
of what they do
product-auto-label
bot
added
size: m
product-auto-label
bot
added
size: l
| done(); | ||
| } catch (e) { | ||
| done(e); | ||
| } |
There was a problem hiding this comment.
The only real change is to make the test runner throw an error if the script throws an error instead of failing silently
test/with-runquery-mockserver.ts
Outdated
| // The error message is based on client library behavior. | ||
| assert.strictEqual( | ||
| (e as Error).message, | ||
| '4 DEADLINE_EXCEEDED: error details: error count: 1', |
There was a problem hiding this comment.
The error message should be something like this. We need to do a bit more investigation into what the client should do in this case to determine the exact error message should be, but for now it's fine to backlog this as a TODO.
| @@ -0,0 +1,126 @@ | |||
| // Copyright 2025 Google LLC | |||
There was a problem hiding this comment.
This file contains utilities for working with the mock server that include a function for producing a deterministic set of errors and a function for shutting down the mock server.
| callback: (arg1: string | null, arg2: {}) => {}, | ||
| ) { | ||
| // SET A BREAKPOINT HERE AND EXPLORE `call` TO SEE THE REQUEST. | ||
| callback(null, {message: 'Hello'}); |
There was a problem hiding this comment.
This grpcEndpoint function has just been moved inside the only place it is used.
danieljbruce
changed the title
test/mock-server-tester.ts
Outdated
| * `DEADLINE_EXCEEDED` code (4), a details message indicating the number of | ||
| * errors generated so far by this instance, and some metadata. | ||
| * | ||
| * @returns {ServiceError} A `ServiceError` object representing a simulated | ||
| * gRPC error. | ||
| */ | ||
| generateError() { |
There was a problem hiding this comment.
nit - without reading the comment, it's actually hard interpret generateError to generate a DEADLINE_EXCEEDED error.
There was a problem hiding this comment.
I restructured this so that we pass the error code into generateError. This might make the intent of this method more clear.
| @@ -0,0 +1,51 @@ | |||
| // Copyright 2025 Google LLC | |||
There was a problem hiding this comment.
nit - the test file is also a bit confusing. I think your intention is to differentiate the test between non-retryable error like NOT_FOUND v.s. a DEADLINE_EXCEEDED error.
But the createreadstream v.s. runquery in the name made it confusing as it seems to do with the functionalities.
There was a problem hiding this comment.
The intent is to differentiate between the calls to the lookup grpc endpoint and the runQuery grpc endpoint.
Note that this PR is in a Draft stage. Although the early feedback is appreciated, I think I'm going to make some larger adjustments like removing this file so that we can lock in the progress we have with the UNAVAILABLE test on runQuery. This gives us the confidence that we can see the error stack for these types of calls.
Number of errors won’t be deterministic before a timeout occurs
Description
The main intent of this PR is to supply a test confirming that users can see the changes introduced by this PR. Primarily, the runQuery test provides an error stack that users can see so that they can identify the root cause of the timeout errors they are seeing. Confirming this behaviour is in place with a test is crucial for solving issues like #1176 going forward.
Impact
This is a crucial step towards solving #1176 giving us the tools we need to explore client library behaviour when the error from the issue occurs.
Testing
This PR only adds tests and no source code changes. The tests are skipped because they fail right now and the goal is to figure out the source code changes that would allow them to pass.
Additional Information
Some related PRs:
googleapis/gax-nodejs#1740
googleapis/gax-nodejs#1650
Next Steps