Skip to content
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
a6d82ef
send error back in the mock server
danieljbruce May 6, 2025
b09bab3
Mock server error tests with fixes
danieljbruce May 6, 2025
68c888b
Change the source code back - no problems
danieljbruce May 6, 2025
e9c0add
Add some console logs
danieljbruce May 6, 2025
57e5064
Add mechanism for shutting down the server
danieljbruce May 6, 2025
cbf59bf
Move the shutdown server function
danieljbruce May 6, 2025
eb61f46
Rename the service method to sendNonRetryableError
danieljbruce May 6, 2025
19fbec5
Delete shutdown comment
danieljbruce May 6, 2025
4c230a1
Only run the run query test
danieljbruce May 6, 2025
30a7838
Eliminate unused import
danieljbruce May 6, 2025
ae6f01c
Complete reproduction of the timeout error
danieljbruce May 6, 2025
e36bd3d
Rename the file to more accurately describe its
danieljbruce May 6, 2025
60303d6
Change the error message
danieljbruce May 6, 2025
0005a22
Add a class for containing the error series
danieljbruce May 6, 2025
a9bf0df
More clear intent in test
danieljbruce May 6, 2025
941c6eb
skip the test
danieljbruce May 6, 2025
2b4691d
Update tests with more accurate descriptions
danieljbruce May 6, 2025
1eacc70
Cleanup the types everywhere
danieljbruce May 6, 2025
8683a77
Modify the test to use UNAVAILABLE
danieljbruce May 7, 2025
341e37e
rename test file
danieljbruce May 7, 2025
e8b9ddc
Delete files to be left out of the PR for now
danieljbruce May 7, 2025
ec849ca
Fix the checks in the test
danieljbruce May 7, 2025
6031197
better documentation
danieljbruce May 7, 2025
1dbbc02
cast as any
danieljbruce May 7, 2025
6a64eae
solve linting errors
danieljbruce May 7, 2025
0f8fb72
Shorter timeout call
danieljbruce May 7, 2025
d726c44
Merge branch 'main' of https://github.com/googleapis/nodejs-datastore…
danieljbruce May 8, 2025
5bdd6bc
Add a couple of console logs
danieljbruce May 8, 2025
e7c0f43
Reduce timeout
danieljbruce May 22, 2025
c8312af
Refactor regex pattern
danieljbruce May 22, 2025
5b1bcaf
Factor replace value
danieljbruce May 22, 2025
f144fd5
Merge branch 'main' into 414574369-test-error-stack-end-to-end
danieljbruce May 22, 2025
d5b3e63
Provide a projectId
danieljbruce May 22, 2025
6899406
Merge branch 'main' of https://github.com/googleapis/nodejs-datastore…
danieljbruce May 22, 2025
ee1c0cd
Merge branch '414574369-test-error-stack-end-to-end' of https://githu…
danieljbruce May 22, 2025
7609fc0
Revert import
danieljbruce May 22, 2025
676d561
move grpc service definition back to where it was
danieljbruce May 22, 2025
9c2f9a7
Thread ResponseType parameter through server confg
danieljbruce May 22, 2025
015e5a6
Remove SuccessType
danieljbruce May 22, 2025
69dbb30
delete file - not used
danieljbruce May 22, 2025
a5f9730
Revert "delete file - not used"
danieljbruce May 22, 2025
5dbbefc
linting fix
danieljbruce May 22, 2025
efa7631
Replace any in call type
danieljbruce May 22, 2025
e1e6599
Eliminate sendNonRetryableError, it is not used
danieljbruce May 22, 2025
8ffcea1
Revert try-server changes
danieljbruce May 22, 2025
e36357d
remove console errors and console logs
danieljbruce May 22, 2025
a36a7de
Eliminate some comments
danieljbruce May 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions mock-server/datastore-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {ServiceError} from 'google-gax';

Check failure on line 15 in mock-server/datastore-server.ts

View workflow job for this annotation

GitHub Actions / lint

"google-gax" is extraneous
import {Server} from '@grpc/grpc-js';

const {dirname, resolve} = require('node:path');

const PROTO_PATH = __dirname + '/../protos/google/datastore/v1/datastore.proto';
Expand All @@ -37,25 +40,38 @@
});
const descriptor = grpc.loadPackageDefinition(packageDefinition);

/**
* Implements the runQuery RPC method.
*/
function grpcEndpoint(
call: {},
callback: (arg1: string | null, arg2: {}) => {},
) {
// SET A BREAKPOINT HERE AND EXPLORE `call` TO SEE THE REQUEST.
callback(null, {message: 'Hello'});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This grpcEndpoint function has just been moved inside the only place it is used.

export type CallType = any;

Check warning on line 43 in mock-server/datastore-server.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
export type SuccessType = any;

Check warning on line 44 in mock-server/datastore-server.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
export type GrpcErrorType = ServiceError | null;

type MockServiceCallback = (arg1: GrpcErrorType, arg2: SuccessType) => {};

interface MockServiceConfiguration {
[endpoint: string]: (call: CallType, callback: MockServiceCallback) => void;
}

/**
* Starts an RPC server that receives requests for datastore
*/
export function startServer(cb: () => void) {
export function startServer(
cb: () => void,
serviceConfigurationOverride?: MockServiceConfiguration,
): Server {
/**
* Implements the runQuery RPC method.
*/
function grpcEndpoint(call: CallType, callback: MockServiceCallback) {
// SET A BREAKPOINT HERE AND EXPLORE `call` TO SEE THE REQUEST.
callback(null, {message: 'Hello'});
}

const server = new grpc.Server();
const service = descriptor.google.datastore.v1.Datastore.service;
// On the next line, change runQuery to the grpc method you want to investigate
server.addService(service, {runQuery: grpcEndpoint});
const serviceConfiguration = serviceConfigurationOverride ?? {
runQuery: grpcEndpoint,
};
server.addService(service, serviceConfiguration);
server.bindAsync(
'0.0.0.0:50051',
grpc.ServerCredentials.createInsecure(),
Expand All @@ -64,4 +80,5 @@
cb();
},
);
return server;
}
126 changes: 126 additions & 0 deletions test/mock-server-tester.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Copyright 2025 Google LLC
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {
CallType,
GrpcErrorType,
SuccessType,
} from '../mock-server/datastore-server';

const grpc = require('@grpc/grpc-js');

Check failure on line 21 in test/mock-server-tester.ts

View workflow job for this annotation

GitHub Actions / lint

"@grpc/grpc-js" is extraneous
import {ServiceError} from 'google-gax';
import {Server} from '@grpc/grpc-js';

Check failure on line 23 in test/mock-server-tester.ts

View workflow job for this annotation

GitHub Actions / lint

"@grpc/grpc-js" is extraneous

/**
* Simulates a non-retryable error response from a gRPC service.
*
* This function is designed for use in mock server setups during testing.
* When invoked as a gRPC endpoint handler, it responds with a pre-configured
* error object representing a non-retryable error. The error includes a
* NOT_FOUND code (5), a fixed details message, and some metadata.
*
* @param {any} call - The gRPC call object, representing the incoming request.
* @param {function} callback - The callback function to be invoked with the
* simulated error response. It expects two arguments: an error object and an
* empty object (representing no response data).
*/
export function sendNonRetryableError(
call: CallType,
callback: (arg1: GrpcErrorType, arg2: SuccessType) => {},
) {
const metadata = new grpc.Metadata();
metadata.set(
'grpc-server-stats-bin',
Buffer.from([0, 0, 116, 73, 159, 3, 0, 0, 0, 0]),
);
const error = new Error('error message') as ServiceError;
error.code = 5;
error.details = 'error details';
error.metadata = metadata;
callback(error, {});
}

export class ErrorGenerator {
private errorSeriesCount = 0;

/**
* Generates an error object for testing purposes.
*
* This method creates a `ServiceError` object, simulating an error
* that might be returned by a gRPC service. The error includes a
* `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() {

Choose a reason for hiding this comment

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

nit - without reading the comment, it's actually hard interpret generateError to generate a DEADLINE_EXCEEDED error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I restructured this so that we pass the error code into generateError. This might make the intent of this method more clear.

// SET A BREAKPOINT HERE AND EXPLORE `call` TO SEE THE REQUEST.
this.errorSeriesCount++;
const metadata = new grpc.Metadata();
metadata.set(
'grpc-server-stats-bin',
Buffer.from([0, 0, 116, 73, 159, 3, 0, 0, 0, 0]),

Choose a reason for hiding this comment

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

what is this buffer?

Choose a reason for hiding this comment

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

+1

);
const error = new Error('error message') as ServiceError;
error.code = 4;
error.details = `error details: error count: ${this.errorSeriesCount}`;
error.metadata = metadata;
return error;
}

/**
* Returns a function that simulates an error response from a gRPC service.
*
* This method is designed to be used in mock server setups for testing purposes.
* It returns a function that, when called, will invoke a callback with a
* pre-configured error object, simulating a gRPC service responding with an error.
* The error includes a DEADLINE_EXCEEDED code (4), a details message indicating the

Choose a reason for hiding this comment

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

why DEADLINE_EXCEEDED? Doesn't it take an arbitrary error code?

* number of errors generated so far by this instance, and some metadata.
*
* @returns {function} A function that takes a `call` object (representing the
* gRPC call) and a `callback` function, and responds to the call with a
* simulated error.
*/
sendErrorSeries() {
return (
call: CallType,
callback: (arg1: GrpcErrorType, arg2: SuccessType) => {},
) => {
const error = this.generateError();
callback(error, {});
};
}
}

export function shutdownServer(server: Server) {
return new Promise((resolve, reject) => {
// Assuming 'server.tryShutdown' is a function that takes a callback.
// The callback is expected to be called when the shutdown attempt is complete.
// If 'tryShutdown' itself can throw an error or indicate an immediate failure
// (without calling the callback), you might need to wrap this call in a try...catch block.

server.tryShutdown((error: Error | undefined) => {
if (error) {
// If the callback is called with an error, reject the promise.
console.error('Server shutdown failed:', error);
reject(error);
} else {
// If the callback is called without an error, resolve the promise.
console.log('Server has been shut down successfully.');
resolve('done');
}
});
});
}
24 changes: 14 additions & 10 deletions test/try-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,21 @@ import {Datastore} from '../src';

import {startServer} from '../mock-server/datastore-server';

describe('Try server', () => {
it.skip('should try to connect to the running server', done => {
describe.skip('Try server', () => {
it('should try to connect to the running server', done => {
startServer(async () => {
const datastore = new Datastore({
namespace: `${Date.now()}`,
apiEndpoint: 'localhost:50051',
});
const postKey = datastore.key(['Post', 'post1']);
const query = datastore.createQuery('Post').hasAncestor(postKey);
const allResults = await datastore.runQuery(query);
done();
try {
const datastore = new Datastore({
namespace: `${Date.now()}`,
apiEndpoint: 'localhost:50051',
});
const postKey = datastore.key(['Post', 'post1']);
const query = datastore.createQuery('Post').hasAncestor(postKey);
const allResults = await datastore.runQuery(query);
done();
} catch (e) {
done(e);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only real change is to make the test runner throw an error if the script throws an error instead of failing silently

});
});
});
51 changes: 51 additions & 0 deletions test/with-createreadstream-mockserver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2025 Google LLC

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@danieljbruce danieljbruce May 7, 2025

Choose a reason for hiding this comment

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

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.

//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {describe, it} from 'mocha';
import {Datastore} from '../src';
import * as assert from 'assert';

import {startServer} from '../mock-server/datastore-server';
import {sendNonRetryableError} from './mock-server-tester';

describe.skip('Should make calls to lookup', () => {
it('should report a NOT_FOUND error to the user when it occurs', done => {
startServer(
async () => {
try {
try {
const datastore = new Datastore({
namespace: `${Date.now()}`,
apiEndpoint: 'localhost:50051',
});
const postKey = datastore.key(['Post', 'post1']);
await datastore.get(postKey);
console.log('call failed');
assert.fail('The call should not have succeeded');
} catch (e) {
// The test should produce the right error message here for the user.
assert.strictEqual(
(e as Error).message,
'5 NOT_FOUND: error details',
);
done();
}
} catch (e) {
done(e);
}
},
{lookup: sendNonRetryableError},
);
});
});
56 changes: 56 additions & 0 deletions test/with-runquery-mockserver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2025 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {describe, it} from 'mocha';
import {Datastore} from '../src';
import * as assert from 'assert';

import {startServer} from '../mock-server/datastore-server';
import {ErrorGenerator, shutdownServer} from './mock-server-tester';

describe.skip('Should make calls to runQuery', () => {
it('should report a DEADLINE_EXCEEDED error to the user when it occurs with the original error details', done => {
const errorGenerator = new ErrorGenerator();
const server = startServer(
async () => {
try {
try {
const datastore = new Datastore({
namespace: `${Date.now()}`,
apiEndpoint: 'localhost:50051',
});
const postKey = datastore.key(['Post', 'post1']);
const query = datastore.createQuery('Post').hasAncestor(postKey);
await datastore.runQuery(query);
console.log('call failed');
assert.fail('The call should not have succeeded');
} catch (e) {
// The test should produce the right error message here for the user.
// TODO: Later on we are going to decide on what the error message should be
// The error message is based on client library behavior.
assert.strictEqual(
(e as Error).message,
'4 DEADLINE_EXCEEDED: error details: error count: 1',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

);
await shutdownServer(server);
done();
}
} catch (e) {
done(e);
}
},
{runQuery: errorGenerator.sendErrorSeries()},
);
});
});
Loading