-
Notifications
You must be signed in to change notification settings - Fork 286
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
feat(connector-fabric): add WatchBlocks endpoint #2119
feat(connector-fabric): add WatchBlocks endpoint #2119
Conversation
13634d5
to
7bdb18b
Compare
OK, it seems that fabric SDK upgrade is not as trivial as I wanted it to be because it breaks all the custom identity providers feature tests:
When using 2.3.0-snapshot.69 or later (including 2.5 I've initially used) there's following error during runtime: When using LTS 2.2 (which also solves my test), the fabric connector doesn't compile, because CryptoSuite ` in LTS is still sync instead of async. I couldn't make our identity
@petermetz @jagpreetsinghsasan any thoughts? |
...-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
Outdated
Show resolved
Hide resolved
...-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
Show resolved
Hide resolved
...-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
Outdated
Show resolved
Hide resolved
...-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
Outdated
Show resolved
Hide resolved
...-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
Outdated
Show resolved
Hide resolved
...-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
Outdated
Show resolved
Hide resolved
@outSH |
I read the same thing on the internet, but I've updated the test ledger to latest 2.4, to match fabric-sdk from this package and the problem remains (and only for identity related stuff). Or do you have different setup in mind?? |
7bdb18b
to
e936960
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@outSH can you re-request review from @petermetz if the requested changes were addressed? |
69dd303
to
1c3dbf2
Compare
@petermetz Funny thing, I've just needed to update fabric SDK on test ledger as well, it somehow interfered with these problematic tests. All works now, ready to merge :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read the same thing on the internet, but I've updated the test ledger to latest 2.4, to match fabric-sdk from this package and the problem remains (and only for identity related stuff). Or do you have different setup in mind??
@petermetz Funny thing, I've just needed to update fabric SDK on test ledger as well, it somehow interfered with these problematic tests. All works now, ready to merge :)
@outSH Sorry for the slow response! Very glad to hear it figured itself out somehow! Please resolve the merge conflicts and then pass it back for review!
1c3dbf2
to
1f7b676
Compare
|
Suggested edit: diff --git a/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts b/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
index 82bac1f0..15f91a5b 100644
--- a/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
+++ b/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/watch-blocks/watch-blocks-v1-endpoint.ts
@@ -19,6 +19,7 @@ import {
import safeStringify from "fast-safe-stringify";
import sanitizeHtml from "sanitize-html";
+import { RuntimeError } from "run-time-error";
/**
* WatchBlocksV1Endpoint configuration.
@@ -254,11 +255,9 @@ export class WatchBlocksV1Endpoint {
listenerType = "full";
break;
default:
- // Will not compile if any type was not handled by above switch.
- const unknownType: never = type;
- throw new Error(
- `Unknown block listen type - '${unknownType}'. Check name and connector version.`,
- );
+ const validTypes = Object.keys(WatchBlocksListenerTypeV1).join(";");
+ const errorMessage = `Unknown block listen type '${type}'. Check name and connector version. Accepted listener types for WatchBlocksListenerTypeV1 are: [${validTypes}]`;
+ throw new RuntimeError(errorMessage);
}
return { listener, listenerType };
diff --git a/packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v2-2-x/fabric-watch-blocks-v1-endpoint.test.ts b/packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v2-2-x/fabric-watch-blocks-v1-endpoint.test.ts
index bb713579..92941212 100644
--- a/packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v2-2-x/fabric-watch-blocks-v1-endpoint.test.ts
+++ b/packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v2-2-x/fabric-watch-blocks-v1-endpoint.test.ts
@@ -400,4 +400,31 @@ describe("watchBlocksV1 of fabric connector tests", () => {
await monitorPromise;
});
+
+ test("Invalid WatchBlocksListenerTypeV1 value gets knocked down", async () => {
+ const monitorPromise = testWatchBlock(
+ "CactusTransactionsTest",
+ "Some_INVALID_WatchBlocksListenerTypeV1" as WatchBlocksListenerTypeV1,
+ () => undefined, // will never reach this because it is meant to error out
+ );
+
+ try {
+ await monitorPromise;
+ } catch (ex: unknown) {
+ // Execution never reaches this point - I'm assuming because the
+ // testWatchBlock method somehow does not fulfil it's obligation of
+ // either succeeding or throwing (it seems to get stuck idling forever
+ // when I debug this in VSCode)
+ expect(ex).toBeTruthy();
+ expect(ex).toBe(Error);
+ expect(ex).toHaveProperty("message");
+ expect((ex as Error).message).toContainAllValues([
+ WatchBlocksListenerTypeV1.CactusTransactions,
+ WatchBlocksListenerTypeV1.Filtered,
+ WatchBlocksListenerTypeV1.Full,
+ WatchBlocksListenerTypeV1.Private,
+ ...Object.keys(WatchBlocksListenerTypeV1),
+ ]);
+ }
+ });
});
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@outSH Please see #2119 (comment) and then pass it back for review after you decided how to proceed.
- Add new endpoint for monitoring new blocks. Response type is determined from input argument. Monitor endpoints are stored in connector.runningWatchBlocksMonitors. - Add `FabricApiClient` which supports new socketio endpoint. - Add functional test to check the new endpoint. - Upgrade fabric-sdk-node to solve test hand issue. - Loose up `ISocketApiClient` to allow interface monitor options. - Update readme with WatchBlocks usage. Regarding fabric-sdk-node upgrade: with sdk2.3, the test used to hang occasionally. I've ensured that all resources are being cleaned-up, and investigated the problem. The issue was infinite recurse dns resolution (of fabric peer address) in grpc-js `ResolvingLoadBalancer`. Could not find exact cause of this strange behaviour, the code there is pretty complex and didn't want to dig to deep, but I've noticed that recently there were some fixes in the functions I suspected so I've updated the fabric-sdk, which uses newer grpc-js, and the problem went away, so I assume this was in fact bug in gprc-js. Run this test on repeat for hours and didn't notice any error. Closes: hyperledger-cacti#2118 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
1f7b676
to
6c62de4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@outSH LGTM, thank you!
Monitor endpoints are stored in connector.runningWatchBlocksMonitors.
FabricApiClient
which supports new socketio endpoint.ISocketApiClient
to allow interface monitor options.Regarding fabric-sdk-node upgrade: with sdk2.3, the test used to hang occasionally.
I've ensured that all resources are being cleaned-up, and investigated the problem. The issue was
infinite recurse dns resolution (of fabric peer address) in grpc-js
ResolvingLoadBalancer
.Could not find exact cause of this strange behaviour, the code there is pretty complex
and didn't want to dig to deep, but I've noticed that recently there were some fixes
in the functions I suspected so I've updated the fabric-sdk, which uses newer grpc-js,
and the problem went away, so I assume this was in fact bug in gprc-js. Run this test on repeat for
hours and didn't notice any error.
Closes: #2118
Signed-off-by: Michal Bajer michal.bajer@fujitsu.com