From 42d982dd691a9f1b3fbc958d75402d0130a678aa Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 30 Aug 2023 15:56:06 +0100 Subject: [PATCH] `OutgoingRequestProcessor`: do not throw errors if shutting down (#3683) * `OutgoingRequestProcessor`: do not throw errors if shutting down * Optimised builds throw a different error --- .../OutgoingRequestProcessor.spec.ts | 39 ++++++++++++++++++- src/rust-crypto/OutgoingRequestProcessor.ts | 15 ++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/spec/unit/rust-crypto/OutgoingRequestProcessor.spec.ts b/spec/unit/rust-crypto/OutgoingRequestProcessor.spec.ts index 00b7f6148..5ea91e028 100644 --- a/spec/unit/rust-crypto/OutgoingRequestProcessor.spec.ts +++ b/spec/unit/rust-crypto/OutgoingRequestProcessor.spec.ts @@ -29,8 +29,9 @@ import { } from "@matrix-org/matrix-sdk-crypto-wasm"; import { TypedEventEmitter } from "../../../src/models/typed-event-emitter"; -import { HttpApiEvent, HttpApiEventHandlerMap, MatrixHttpApi, UIAuthCallback } from "../../../src"; +import { HttpApiEvent, HttpApiEventHandlerMap, IHttpOpts, MatrixHttpApi, UIAuthCallback } from "../../../src"; import { OutgoingRequestProcessor } from "../../../src/rust-crypto/OutgoingRequestProcessor"; +import { defer } from "../../../src/utils"; describe("OutgoingRequestProcessor", () => { /** the OutgoingRequestProcessor implementation under test */ @@ -218,4 +219,40 @@ describe("OutgoingRequestProcessor", () => { await Promise.all([processor.makeOutgoingRequest(outgoingRequest), markSentCallPromise]); expect(olmMachine.markRequestAsSent).toHaveBeenCalledWith("5678", 987, ""); }); + + it("does not explode if the OlmMachine is stopped while the request is in flight", async () => { + // we use a real olm machine for this test + const olmMachine = await RustSdkCryptoJs.OlmMachine.initialize( + new RustSdkCryptoJs.UserId("@alice:example.com"), + new RustSdkCryptoJs.DeviceId("TEST_DEVICE"), + ); + + const authRequestResultDefer = defer(); + + const authRequestCalledPromise = new Promise((resolve) => { + const mockHttpApi = { + authedRequest: async () => { + resolve(); + return await authRequestResultDefer.promise; + }, + } as unknown as Mocked>; + processor = new OutgoingRequestProcessor(olmMachine, mockHttpApi); + }); + + // build a request + const request = olmMachine.queryKeysForUsers([new RustSdkCryptoJs.UserId("@bob:example.com")]); + const result = processor.makeOutgoingRequest(request); + + // wait for the HTTP request to be made + await authRequestCalledPromise; + + // while the HTTP request is in flight, the OlmMachine gets stopped. + olmMachine.close(); + + // the HTTP request completes... + authRequestResultDefer.resolve("{}"); + + // ... and `makeOutgoingRequest` resolves satisfactorily + await result; + }); }); diff --git a/src/rust-crypto/OutgoingRequestProcessor.ts b/src/rust-crypto/OutgoingRequestProcessor.ts index d5d12468b..ae5ee4465 100644 --- a/src/rust-crypto/OutgoingRequestProcessor.ts +++ b/src/rust-crypto/OutgoingRequestProcessor.ts @@ -105,7 +105,20 @@ export class OutgoingRequestProcessor { } if (msg.id) { - await this.olmMachine.markRequestAsSent(msg.id, msg.type, resp); + try { + await this.olmMachine.markRequestAsSent(msg.id, msg.type, resp); + } catch (e) { + // Ignore errors which are caused by the olmMachine having been freed. The exact error message depends + // on whether we are using a release or develop build of rust-sdk-crypto-wasm. + if ( + e instanceof Error && + (e.message === "Attempt to use a moved value" || e.message === "null pointer passed to rust") + ) { + logger.log(`Ignoring error '${e.message}': client is likely shutting down`); + } else { + throw e; + } + } } }