Skip to content

Commit

Permalink
Assert against RequestIDError
Browse files Browse the repository at this point in the history
  • Loading branch information
@odeke-em
odeke-em committed Dec 27, 2024
1 parent ee2c506 commit 54d1188
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 0 deletions.
13 changes: 13 additions & 0 deletions src/request_id_header.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ class AtomicCounter {
public toString(): string {
return `${this.value()}`;
}

public reset(value: number) {
Atomics.store(this.backingBuffer, 0, 0);
}
}

function craftRequestId(
Expand All @@ -59,6 +63,11 @@ function craftRequestId(

const nthClientId = new AtomicCounter();

// Only exported for deterministic testing.
export function resetNthClientId() {
nthClientId.reset(0);
}

/*
* nextSpannerClientId increments the internal
* counter for created SpannerClients, for use
Expand Down Expand Up @@ -318,6 +327,10 @@ function nextNthRequest(database): number {
return database._nextNthRequest();
}

export interface RequestIDError extends grpc.ServiceError {
requestID: string;
}

export {
AtomicCounter,
X_GOOG_SPANNER_REQUEST_ID_HEADER,
Expand Down
77 changes: 77 additions & 0 deletions test/spanner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ import {
LEADER_AWARE_ROUTING_HEADER,
} from '../src/common';
import {
RequestIDError,
XGoogRequestHeaderInterceptor,
randIdForProcess,
resetNthClientId,
} from '../src/request_id_header';
import CreateInstanceMetadata = google.spanner.admin.instance.v1.CreateInstanceMetadata;
import QueryOptions = google.spanner.v1.ExecuteSqlRequest.QueryOptions;
Expand Down Expand Up @@ -126,6 +128,7 @@ describe('Spanner with mock server', () => {
}

beforeEach(() => {
resetNthClientId();
xGoogReqIDInterceptor.reset();
});

Expand Down Expand Up @@ -288,6 +291,10 @@ describe('Spanner with mock server', () => {
// Ignore the fact that streaming read is unimplemented on the mock
// server. We just want to verify that the correct request is sent.
assert.strictEqual((e as ServiceError).code, Status.UNIMPLEMENTED);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.3.1`
);
} finally {
snapshot.end();
await database.close();
Expand Down Expand Up @@ -447,6 +454,10 @@ describe('Spanner with mock server', () => {
// Ignore the fact that streaming read is unimplemented on the mock
// server. We just want to verify that the correct request is sent.
assert.strictEqual((e as ServiceError).code, Status.UNIMPLEMENTED);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
return undefined;
} finally {
tx.end();
Expand Down Expand Up @@ -1143,6 +1154,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
'2 UNKNOWN: Test error'
);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
} finally {
await database.close();
}
Expand Down Expand Up @@ -1197,6 +1212,11 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
'14 UNAVAILABLE: Transient error'
);
// Ensure that we have a requestID returned and it was on the 2nd request.
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
} finally {
await database.close();
}
Expand Down Expand Up @@ -1418,6 +1438,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
'2 UNKNOWN: Test error'
);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
}
await database.close();
});
Expand Down Expand Up @@ -1456,6 +1480,10 @@ describe('Spanner with mock server', () => {
database.run(selectSql, err => {
assert.ok(err, 'Missing expected error');
assert.strictEqual(err!.message, '2 UNKNOWN: Non-retryable error');
assert.deepStrictEqual(
(err as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
database
.close()
.catch(done)
Expand All @@ -1479,6 +1507,10 @@ describe('Spanner with mock server', () => {
.on('error', err => {
assert.strictEqual(err.message, '2 UNKNOWN: Non-retryable error');
assert.strictEqual(receivedRows.length, index);
assert.deepStrictEqual(
(err as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
database
.close()
.catch(done)
Expand Down Expand Up @@ -1559,6 +1591,10 @@ describe('Spanner with mock server', () => {
attempts++;
tx!.runUpdate(insertSql, err => {
assert.ok(err, 'Missing expected error');
assert.deepStrictEqual(
(err as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
assert.strictEqual(err!.code, grpc.status.INVALID_ARGUMENT);
// Only the update RPC should be retried and not the entire
// transaction.
Expand Down Expand Up @@ -2566,6 +2602,7 @@ describe('Spanner with mock server', () => {

it('should reuse sessions after executing invalid sql', async () => {
// The query to execute
const requestIDRegex = new RegExp(`1.${randIdForProcess}.1.1.\\d+.1`);
const query = {
sql: invalidSql,
};
Expand All @@ -2581,6 +2618,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
`${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
);
assert.deepStrictEqual(
(e as RequestIDError).requestID.match(requestIDRegex) != null,
true
);
}
}
assert.strictEqual(pool.size, 1);
Expand Down Expand Up @@ -2609,6 +2650,7 @@ describe('Spanner with mock server', () => {

it('should reuse sessions after executing an invalid streaming sql', async () => {
// The query to execute
const requestIDRegex = new RegExp(`1.${randIdForProcess}.1.1.\\d+.1`);
const query = {
sql: invalidSql,
};
Expand All @@ -2624,6 +2666,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
`${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
);
assert.deepStrictEqual(
(e as RequestIDError).requestID.match(requestIDRegex) != null,
true
);
}
}
assert.strictEqual(pool.size, 1);
Expand Down Expand Up @@ -2702,6 +2748,7 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
'No resources available.'
);
// assert.deepStrictEqual((e as RequestIDError).requestID,`1.${randIdForProcess}.1.1.1.1`);
}
} finally {
if (tx1) {
Expand Down Expand Up @@ -2776,6 +2823,7 @@ describe('Spanner with mock server', () => {
assert.fail('missing expected error');
} catch (err) {
assert.strictEqual((err as ServiceError).code, Status.NOT_FOUND);
// assert.deepStrictEqual((err as RequestIDError).requestID,`1.${randIdForProcess}.1.1.1.1`);
} finally {
await database.close();
}
Expand Down Expand Up @@ -2837,6 +2885,7 @@ describe('Spanner with mock server', () => {
(err as ServiceError).code,
Status.PERMISSION_DENIED
);
// assert.deepStrictEqual((err as RequestIDError).requestID,`1.${randIdForProcess}.1.1.1.1`);
} finally {
await database.close();
}
Expand Down Expand Up @@ -3247,6 +3296,10 @@ describe('Spanner with mock server', () => {
assert.ok(
(err as ServiceError).message.includes('Generic internal error')
);
assert.deepStrictEqual(
(err as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.3.1`
);
} finally {
await database.close();
}
Expand Down Expand Up @@ -3359,6 +3412,10 @@ describe('Spanner with mock server', () => {
} catch (err) {
assert(err, 'Expected an error to be thrown');
assert.match((err as Error).message, /Table FOO not found/);
assert.deepStrictEqual(
(err as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.3.1`
);
}
});
});
Expand Down Expand Up @@ -3782,6 +3839,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
`${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
}
await tx.run(selectSql);
await tx.commit();
Expand Down Expand Up @@ -3839,6 +3900,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
`${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
}
await tx.run(selectSql);
await tx.commit();
Expand Down Expand Up @@ -3867,6 +3932,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
`${grpc.status.NOT_FOUND} NOT_FOUND: ${fooNotFoundErr.message}`
);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
}
await tx.run(selectSql);
await tx.commit();
Expand Down Expand Up @@ -3907,6 +3976,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
'2 UNKNOWN: Test error'
);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.4.1`
);
} finally {
await database.close();
}
Expand Down Expand Up @@ -3995,6 +4068,10 @@ describe('Spanner with mock server', () => {
(e as ServiceError).message,
'2 UNKNOWN: Test error'
);
assert.deepStrictEqual(
(e as RequestIDError).requestID,
`1.${randIdForProcess}.1.1.2.1`
);
} finally {
await database.close();
}
Expand Down

0 comments on commit 54d1188

Please sign in to comment.