Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Commit

Permalink
tidy up
Browse files Browse the repository at this point in the history
  • Loading branch information
jeffsmale90 committed Aug 5, 2022
1 parent 942bacc commit 3d555b9
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 49 deletions.
7 changes: 3 additions & 4 deletions src/chains/ethereum/ethereum/src/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,17 +420,16 @@ export class EthereumProvider

/**
* Disconnect the provider instance. This will cause the underlying blockchain to be stopped, and any pending
* tasks to be rejected. Await the returned Promise to ensure that everything has been cleanly shut down before
* terminating the process.
* @return Promise<void> - indicating that the provider has been cleanly disconnected
* tasks to be rejected. Emits a `disconnect` event once successfully disconnected.
* @returns Fullfills with `undefined` once the provider has been disconnected.
*/
public disconnect = async () => {
// executor.stop() will stop accepting new tasks, but will not wait for inflight tasks. These may reject with
// (unhelpful) internal errors. See https://github.com/trufflesuite/ganache/issues/3499
this.#executor.stop();
await this.#blockchain.stop();

this.#executor.rejectPendingTasks();
this.#executor.end();
this.emit("disconnect");
};

Expand Down
59 changes: 23 additions & 36 deletions src/chains/ethereum/ethereum/tests/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ describe("provider", () => {
});
});

describe.only("disconnect()", () => {
describe("disconnect()", () => {
let provider: EthereumProvider;

[true, false].forEach(asyncRequestProcessing => {
Expand All @@ -473,26 +473,31 @@ describe("provider", () => {
});
});

it("stops responding to RPC methods once disconnected", async () => {
await provider.disconnect();
it("rejects requests after disconnect() is called", async () => {
provider.disconnect();
const whenBlockByNumber = provider.request({
method: "eth_getBlockByNumber",
params: ["latest"]
});

await assert.rejects(
provider.send("eth_getBlockByNumber", ["latest"]),
new Error("Cannot process request, Ganache is disconnected.")
whenBlockByNumber,
new Error("Cannot process request, Ganache is disconnected."),
"Requests made after disconnect is called should reject"
);
});

it("raises the 'disconnect' event", async () => {
it("emits the 'disconnect' event", async () => {
const whenDisconnected = provider.once("disconnect");
await provider.disconnect();
await assert.doesNotReject(
whenDisconnected,
'The provider should raise the "disconnect" event'
'The provider should emit the "disconnect" event'
);
});

// todo: Reinstate this test when https://github.com/trufflesuite/ganache/issues/3499 is fixed
it.skip("successfully processes requests executed before disconnect is called", async () => {
it.skip("processes requests executed before disconnect is called", async () => {
const whenBlockByNumber = provider.request({
method: "eth_getProof",
params: ["0xC7D9E2d5FE0Ff5C43102158C31BbC4aA2fDe10d8", [], "latest"]
Expand All @@ -501,46 +506,28 @@ describe("provider", () => {

await assert.doesNotReject(
whenBlockByNumber,
"A call to .request() on the provider before disconnect is called should succeed"
"Currently executing request should resolve"
);
await assert.doesNotReject(
whenDisconnected,
'The provider should raise the "disconnect" event'
);
});

it("rejects requests after disconnect is called", async () => {
const whenDisconnected = provider.disconnect();
const whenBlockByNumber = provider.request({
method: "eth_getBlockByNumber",
params: ["latest"]
});

await assert.rejects(
whenBlockByNumber,
new Error("Cannot process request, Ganache is disconnected.")
);
await assert.doesNotReject(
whenDisconnected,
'The provider should raise the "disconnect" event'
'The provider should emit the "disconnect" event'
);
});
});
});

describe("without asyncRequestProcessing", () => {
beforeEach("Instantiate provider", async () => {
// todo: Reinstate this test when https://github.com/trufflesuite/ganache/issues/3499 is fixed
describe.skip("without asyncRequestProcessing", () => {
// we only test this with asyncRequestProcessing: false, because it's impossible to force requests
// to be "pending" when asyncRequestProcessing: true
it("processes started requests, but reject pending requests", async () => {
provider = await getProvider({
chain: { asyncRequestProcessing: false }
});
});

// we only test this with asyncRequestProcessing: false, because it's impossible to force requests
// to be "pending" when asyncRequestProcessing: true
it("successfully processes started requests, but reject pending requests", async () => {
const active = provider.request({
method: "eth_getBlockByNumber",
params: ["latest"]
method: "eth_getProof",
params: ["0x4Ae2736a3b914C7597131fd1Ef30F74aC4B20874", [], "latest"]
});
const pending = provider.request({
method: "eth_getBlockByNumber",
Expand All @@ -557,7 +544,7 @@ describe("provider", () => {
await assert.doesNotReject(active, "active tasks should not reject");
await assert.doesNotReject(
whenDisconnected,
'The provider should raise the "disconnect" event'
'The provider should emit the "disconnect" event'
);
});
});
Expand Down
7 changes: 5 additions & 2 deletions src/packages/utils/src/utils/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ export class Executor {
this.#requestCoordinator.stop();
}

public rejectPendingTasks() {
this.#requestCoordinator.rejectPendingTasks();
/**
* Finalise shutdown of the underlying RequestCoordinator.
*/
public end() {
this.#requestCoordinator.end();
}

/**
Expand Down
5 changes: 3 additions & 2 deletions src/packages/utils/src/utils/request-coordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,10 @@ export class RequestCoordinator {
}

/**
* All pending tasks will reject with an error indicating that Ganache is disconnected.
* Finalise shutdown of the RequestCoordinator. Rejects all pending tasks in order. Should be
* called after all in-flight tasks have resolved in order to maintain FIFIO order.
*/
public rejectPendingTasks() {
public end() {
for(const current of this.pending) {
current.reject(
new Error("Cannot process request, Ganache is disconnected.")
Expand Down
10 changes: 5 additions & 5 deletions src/packages/utils/tests/request-coordinator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe("request-coordinator", () => {
});
});

describe("rejectAllPendingRequests()", () => {
describe("end()", () => {
it("should reject pending requests in the order that they were received", async () => {
coordinator.pause();

Expand All @@ -54,7 +54,7 @@ describe("request-coordinator", () => {
pendingAssertions.push(assert.rejects(task, new Error("Cannot process request, Ganache is disconnected.")));
}

coordinator.rejectPendingTasks();
coordinator.end();
await Promise.all(pendingAssertions);

assert.equal(coordinator.pending.length, 0, "Coordinator pending list should be empty");
Expand All @@ -67,10 +67,10 @@ describe("request-coordinator", () => {
coordinator.queue(noop, this, []);
}

assert.equal(coordinator.pending.length, 10, "Incorrect pending queue length before calling rejectAllPendingRequests");
assert.equal(coordinator.pending.length, 10, "Incorrect pending queue length before calling end()");

coordinator.rejectPendingTasks();
assert.equal(coordinator.pending.length, 0, "Incorrect pending queue length after calling rejectAllPendingRequests");
coordinator.end();
assert.equal(coordinator.pending.length, 0, "Incorrect pending queue length after calling end()");
});
});
});

0 comments on commit 3d555b9

Please sign in to comment.