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

Multiple fixes and updates to rhea-promise #44

Merged
merged 19 commits into from
Jun 27, 2019
Merged

Conversation

amarzavery
Copy link
Collaborator

@amarzavery amarzavery commented Jun 13, 2019

  • Added a read only property id to the Session object. The id property is created by concatenating session's local channel, remote channel and the connection id "local-<number>_remote-<number>_<connection-id>", thus making it unique for that connection.
  • Improved log statements by adding the session id and the sender, receiver names to help while debugging applications.
  • Added options to Link.close({closeSession: true | false}), thus the user can specify whether the underlying session should be closed while closing the Sender|Receiver. Default is true.
  • Improved open and close operations on Connection, Session and Link by cleating timer in case the connection gets disconnected. Fixes #41.
  • The current Sender does not have a provision of awaiting on sending a message. The user needs to add handlers on the Sender for accepted, rejected, released, modified to ensure whether the message was successfully sent. Now, we have added a new AsynchronousSender which adds the handlers internally and provides an awaitable send() operation to the customer. Fixes #45.
  • Exported new Errors:
    • InsufficientCreditError: Defines the error that occurs when the Sender does not have enough credit.
    • SendOperationFailedError: Defines the error that occurs when the Sender fails to send a message.

@amarzavery
Copy link
Collaborator Author

amarzavery commented Jun 13, 2019

@GaikwadPratik - Can you please

  1. download this rhea-promise-0.2.1.zip

  2. update the dependency for rhea-promise to the location of downloaded zip file in package.json

  3. delete the node_modules folder and package-lock.json file

  4. execute npm install

Run your app and then provide the updated logs.

Note: If the installation is not successful, then please try changing the file extension from .zip to .tgz. Mostly that should work.

If that also does not work then feel to clone the branch from my fork and then run npm i and npm pack which should give the tar ball.

I have added the session id in all the log statements, which should hopefully help us find the root cause, relatively sooner.

lib/link.ts Show resolved Hide resolved
lib/link.ts Outdated Show resolved Hide resolved
@amarzavery
Copy link
Collaborator Author

@ramya-rao-a - Please review.

@amarzavery amarzavery requested review from ramya-rao-a and grs June 14, 2019 00:18
@GaikwadPratik
Copy link

@amarzavery ,

  1. download this rhea-promise-0.2.1.zip
    I did download zip file, fyi npm does not like zip files.

Note: If the installation is not successful, then please try changing the file extension from .zip to .tgz. Mostly that should work.

I did create two packages one from above zip file and one from "rhea-promise": "github:amarzavery/rhea-promise#logs". Just before moving and executing, I want to make sure I've your changes. Can you please point to me one location to verify your changes are in the packed file?

I was trying to verify using this change, but it seems I get function like this

onOpen = (context: RheaEventContext) => {
          removeListeners();
          log.connection("[%s] Resolving the promise with amqp connection.", this.id);
          return resolve(this);
        };

@amarzavery
Copy link
Collaborator Author

amarzavery commented Jun 14, 2019

Hey @GaikwadPratik,

You can download the tar.gz from here.

In package.json provide the absolute path to the tar.gz file

"dependencies": {
    "rhea-promise": "/<absolute path to the tarball>/rhea-promise-0.2.1.tgz"
 }

W.r.t location,
Line 457 in lib/connection.ts should be
log.session("[%s] Resolving the promise with amqp session '%s'.", this.id, session.id);

@GaikwadPratik
Copy link

Hey @GaikwadPratik,

You can download the tar.gz from here.

In package.json provide the absolute path to the tar.gz file

"dependencies": {
    "rhea-promise": "/<absolute path to the tarball>/rhea-promise-0.2.1.tgz"
 }

Is there a specific change I can check to ensure the correct changes are in?

@amarzavery
Copy link
Collaborator Author

amarzavery commented Jun 14, 2019

The error message that you were getting for OperationTimeoutError has been modified.

The updated message as can be seen at this location is:

const msg: string = `Unable to close the ${this.type} '${this.name}' ` +
            `on amqp session '${this.session.id}' due to operation timeout.`;

@amarzavery
Copy link
Collaborator Author

Make sure to delete the package-lock.json file before installing. It can cause issues while installing an update. Deleting the node_modules folder is also a good thing to ensure that you had a clean install.

GaikwadPratik pushed a commit to GaikwadPratik/rhea-rpc that referenced this pull request Jun 15, 2019
@amarzavery amarzavery mentioned this pull request Jun 15, 2019
lib/connection.ts Outdated Show resolved Hide resolved
lib/connection.ts Outdated Show resolved Hide resolved
lib/connection.ts Outdated Show resolved Hide resolved
lib/asynchronousSender.ts Outdated Show resolved Hide resolved
lib/asyncSender.ts Outdated Show resolved Hide resolved
lib/asyncSender.ts Outdated Show resolved Hide resolved
lib/asyncSender.ts Outdated Show resolved Hide resolved
lib/asyncSender.ts Outdated Show resolved Hide resolved
lib/asyncSender.ts Outdated Show resolved Hide resolved
lib/asyncSender.ts Outdated Show resolved Hide resolved
lib/asyncSender.ts Outdated Show resolved Hide resolved
changelog.md Outdated Show resolved Hide resolved
changelog.md Outdated Show resolved Hide resolved
.vscode/launch.json Outdated Show resolved Hide resolved
lib/awaitableSender.ts Outdated Show resolved Hide resolved
lib/awaitableSender.ts Outdated Show resolved Hide resolved
lib/awaitableSender.ts Outdated Show resolved Hide resolved
lib/errorDefinitions.ts Show resolved Hide resolved
lib/link.ts Show resolved Hide resolved
@amarzavery
Copy link
Collaborator Author

@amarzavery amarzavery changed the title Added id to the session object and improved log statements Multiple fixe and updates to rhea-promise Jun 20, 2019
@amarzavery amarzavery changed the title Multiple fixe and updates to rhea-promise Multiple fixes and updates to rhea-promise Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants