Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revamp retry and remove unintended exports #69

Merged
merged 6 commits into from
Apr 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 41 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,44 @@
# 2.2.0
This release is focused on improving the retry logic, optimizing it and handling more possible failures, as well as more
strictly defining the API to help prevent misuse. These changes are potentially breaking if the driver is being used in
a way that isn't supported by the documentation.

## :tada: Enhancements
* Improved retry logic
* Failures when starting a new session are now retried.
* Dead sessions are immediately discarded, reducing latency when using the driver.
* `ClientError`, `DriverClosedError`, `LambdaAbortedError`, and `SessionPoolEmptyError` are now exported.
* Peer dependency `aws-sdk` bumped to `2.841.0` or greater, which gives visibility to `CapacityExceededException`.

## :warning: API Clean-up

These changes either remove unintentionally exported modules or remove the use of `any`. Updating to 2.2.0 should not break any documented usage of the driver and should not require code changes. If something is now incompatible and it's believed that it should be, please open an issue.

* TypeScript: updated `executeLambda` signature

* ```typescript
// Old
async ExecuteLambda(transactionLambda: (transactionExecutor: TransactionExecutor) => any,
retryConfig?: RetryConfig): Promise<any>;
// New
async ExecuteLambda<Type>(transactionLambda: (transactionExecutor: TransactionExecutor) => Promise<Type>,
retryConfig?: RetryConfig): Promise<Type>;
```

* The returned value from the `transactionLambda` is what `ExecuteLambda` returns, so it's now strictly defined by the `Type`.

* The `transactionLambda` must return a `Promise`, as any methods called on the `TransactionExecutor` must be awaited for the driver to properly function.

* JavaScript: removed `QldbDriver.getSession()`

* This is unavailable to TypeScript users and was not intended to be called directly by JavaScript users.

* Module exports

* Removed `Transaction` from the exports list.
* Removed modules being accessible when importing from `amazon-qldb-driver-nodejs/src`.
* TypeScript: Removed constructors in the API for some exported types.

# 2.1.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add the date for when 2.1.1 was released

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 think we stopped doing that. The release date is also available on npm repository.

* Export `ResultReadable`.
* Change the return type of `executeAndStreamResults()` from `Readable` to `ResultReadable` which extends `Readable`.
Expand Down Expand Up @@ -117,4 +158,3 @@ Please check the [release notes](http://github.com/awslabs/amazon-qldb-driver-no
# 0.1.0-preview.1 (2019-11-08)

* Preview release of the driver.

8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ from the ~/.aws/config file.

See [Setting Region](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/setting-region.html) page for more information.

### TypeScript 3.5.x
### TypeScript 3.8.x

Choose a reason for hiding this comment

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

Is 3.8 needed, or can it stay at 3.5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly we don't even need 3.5. The real version we need is some undetermined previous version.

Figured we'd align up our documentation and all and 3.8 was what I was playing with testing hash private functions. This ultimately doesn't matter because it's an independent dev dependency.


The driver is written in, and requires, TypeScript 3.5.x. It will be automatically installed as a dependency.
Please see the link below for more detail on TypeScript 3.5.x:
The driver is written in, and requires, TypeScript 3.8.x. It will be automatically installed as a dependency.
Please see the link below for more detail on TypeScript 3.8.x:

* [TypeScript 3.5.x](https://www.npmjs.com/package/typescript)
* [TypeScript 3.8.x](https://www.npmjs.com/package/typescript)



Expand Down
5 changes: 4 additions & 1 deletion index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
export {
ClientError,
DriverClosedError,
LambdaAbortedError,
SessionPoolEmptyError,
isBadRequestException,
isInvalidParameterException,
isInvalidSessionException,
Expand All @@ -10,7 +14,6 @@ export {
export { QldbDriver } from "./src/QldbDriver";
export { Result } from "./src/Result";
export { ResultReadable } from "./src/ResultReadable";
export { Transaction } from "./src/Transaction";
export { TransactionExecutor } from "./src/TransactionExecutor";
export { RetryConfig } from "./src/retry/RetryConfig";
export { IOUsage } from "./src/stats/IOUsage";
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
},
"name": "amazon-qldb-driver-nodejs",
"description": "The Node.js driver for working with Amazon Quantum Ledger Database",
"version": "2.1.1",
"version": "2.2.0",
"main": "dist/index.js",
"types": "dist/index.d.ts",
"engines": {
Expand All @@ -19,7 +19,7 @@
"@types/sinon": "^7.0.13",
"@typescript-eslint/eslint-plugin": "^2.5.0",
"@typescript-eslint/parser": "^2.5.0",
"aws-sdk": "^2.815.0",
"aws-sdk": "^2.841.0",
"chai": "^4.2.0",
"chai-as-promised": "^7.1.1",
"cross-env": "^6.0.3",
Expand All @@ -34,10 +34,10 @@
"sinon": "^7.3.2",
"ts-node": "^8.3.0",
"typedoc": "^0.15.0",
"typescript": "^3.5.3"
"typescript": "^3.8.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be called out in CHANGELOG?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a dev dependency so it doesn't matter to users

},
"peerDependencies": {
"aws-sdk": "^2.815.0",
"aws-sdk": "^2.841.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason we aren't bumping the devDependency aws-sdk to the same verison?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix

"ion-js": "^4.0.0",
"jsbi": "^3.1.1"
},
Expand Down
24 changes: 10 additions & 14 deletions src/Communicator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
* the License. A copy of the License is located at
Expand All @@ -16,6 +16,7 @@ import {
AbortTransactionResult,
CommitDigest,
CommitTransactionResult,
EndSessionResult,
ExecuteStatementResult,
FetchPageResult,
PageToken,
Expand All @@ -31,21 +32,20 @@ import { debug, warn } from "./LogUtil";
/**
* A class representing an independent session to a QLDB ledger that handles endpoint requests. This class is used in
* {@linkcode QldbDriver} and {@linkcode QldbSession}. This class is not meant to be used directly by developers.
*
* @internal
*/
export class Communicator {
private _qldbClient: QLDBSession;
private _ledgerName: string;
private _sessionToken: string;

/**
* Creates a Communicator.
* @param qldbClient The low level service client.
* @param ledgerName The QLDB ledger name.
* @param sessionToken The initial session token representing the session connection.
*/
private constructor(qldbClient: QLDBSession, ledgerName: string, sessionToken: string) {
private constructor(qldbClient: QLDBSession, sessionToken: string) {
this._qldbClient = qldbClient;
this._ledgerName = ledgerName;
this._sessionToken = sessionToken;
}

Expand All @@ -62,7 +62,7 @@ export class Communicator {
}
};
const result: SendCommandResult = await qldbClient.sendCommand(request).promise();
return new Communicator(qldbClient, ledgerName, result.StartSession.SessionToken);
return new Communicator(qldbClient, result.StartSession.SessionToken);
}

/**
Expand Down Expand Up @@ -122,19 +122,15 @@ export class Communicator {

/**
* Send request to end the independent session represented by the instance of this class.
* @returns Promise which fulfills with void.
* @returns Promise which fulfills with the end session response returned from QLDB.
*/
async endSession(): Promise<void> {
async endSession(): Promise<EndSessionResult> {
const request: SendCommandRequest = {
SessionToken: this._sessionToken,
EndSession: {}
};
try {
await this._sendCommand(request);
} catch (e) {
// We will only log issues ending the session, as QLDB will clean them after a timeout.
warn(`Errors ending session: ${e}.`);
}
const result: SendCommandResult = await this._sendCommand(request);
return result.EndSession;
}

/**
Expand Down
14 changes: 12 additions & 2 deletions src/LogUtil.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
* the License. A copy of the License is located at
Expand All @@ -18,6 +18,8 @@ import { version } from "../package.json";
/**
* Logs a debug level message.
* @param line The message to be logged.
*
* @internal
*/
export function debug(line: string): void {
if (isLoggerSet()) {
Expand All @@ -28,6 +30,8 @@ export function debug(line: string): void {
/**
* Logs an error level message.
* @param line The message to be logged.
*
* @internal
*/
export function error(line: string): void {
if (isLoggerSet()) {
Expand All @@ -38,6 +42,8 @@ export function error(line: string): void {
/**
* Logs an info level message.
* @param line The message to be logged.
*
* @internal
*/
export function info(line: string): void {
if (isLoggerSet()) {
Expand All @@ -48,13 +54,15 @@ export function info(line: string): void {
/**
* @returns A boolean indicating whether a logger has been set within the AWS SDK.
*/
Copy link
Contributor

@byronlin13 byronlin13 Apr 15, 2021

Choose a reason for hiding this comment

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

do we need @internal here?

Same with _prepend

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because otherwise these are exported to the API

export function isLoggerSet(): boolean {
function isLoggerSet(): boolean {
return config.logger !== null;
}

/**
* Logs a message.
* @param line The message to be logged.
*
* @internal
*/
export function log(line: string): void {
if (isLoggerSet()) {
Expand All @@ -65,6 +73,8 @@ export function log(line: string): void {
/**
* Logs a warning level message.
* @param line The message to be logged.
*
* @internal
*/
export function warn(line: string): void {
if (isLoggerSet()) {
Expand Down
Loading