From 468b4cfe5fae7fee550072802f7e14f77bc5cddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Torres?= <30977845+Torres-ssf@users.noreply.github.com> Date: Fri, 8 Mar 2024 14:44:57 -0300 Subject: [PATCH] chore: add try catch when parsing stream response (#1839) --- .changeset/brave-phones-relax.md | 6 ++ .../src/providers/fuel-graphql-subscriber.ts | 14 +++- .../account/src/providers/provider.test.ts | 71 +++++++++++++++++++ packages/errors/src/error-codes.ts | 3 + .../src/transaction-response.test.ts | 2 +- 5 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 .changeset/brave-phones-relax.md diff --git a/.changeset/brave-phones-relax.md b/.changeset/brave-phones-relax.md new file mode 100644 index 00000000000..8aab721b846 --- /dev/null +++ b/.changeset/brave-phones-relax.md @@ -0,0 +1,6 @@ +--- +"@fuel-ts/account": patch +"@fuel-ts/errors": patch +--- + +Add try/catch block when parsing GraphQL stream data response diff --git a/packages/account/src/providers/fuel-graphql-subscriber.ts b/packages/account/src/providers/fuel-graphql-subscriber.ts index 5741ebca7d1..acabd3b6c5a 100644 --- a/packages/account/src/providers/fuel-graphql-subscriber.ts +++ b/packages/account/src/providers/fuel-graphql-subscriber.ts @@ -1,4 +1,4 @@ -import { FuelError } from '@fuel-ts/errors'; +import { ErrorCode, FuelError } from '@fuel-ts/errors'; import type { DocumentNode } from 'graphql'; import { print } from 'graphql'; @@ -61,7 +61,17 @@ export class FuelGraphqlSubscriber implements AsyncIterator { continue; } - const { data, errors } = JSON.parse(text.split('data:')[1]); + let data; + let errors; + + try { + ({ data, errors } = JSON.parse(text.replace(/^data:/, ''))); + } catch (e) { + throw new FuelError( + ErrorCode.STREAM_PARSING_ERROR, + `Error while parsing stream data response: ${text}` + ); + } if (Array.isArray(errors)) { throw new FuelError( diff --git a/packages/account/src/providers/provider.test.ts b/packages/account/src/providers/provider.test.ts index fa49fbce69c..b9e3052ef6c 100644 --- a/packages/account/src/providers/provider.test.ts +++ b/packages/account/src/providers/provider.test.ts @@ -25,6 +25,7 @@ import type { } from './transaction-request'; import { ScriptTransactionRequest, CreateTransactionRequest } from './transaction-request'; import { TransactionResponse } from './transaction-response'; +import type { SubmittedStatus } from './transaction-summary/types'; import { sleep } from './utils'; import * as gasMod from './utils/gas'; @@ -1039,6 +1040,76 @@ describe('Provider', () => { await Promise.all(promises); }); + it('should not throw if the subscription stream data string contains more than one "data:"', async () => { + const provider = await Provider.create(FUEL_NETWORK_URL); + + vi.spyOn(global, 'fetch').mockImplementationOnce(() => { + const responseObject = { + data: { + submitAndAwait: { + type: 'SuccessStatus', + time: 'data: 4611686020137152060', + }, + }, + }; + const streamedResponse = new TextEncoder().encode( + `data:${JSON.stringify(responseObject)}\n\n` + ); + return Promise.resolve( + new Response( + new ReadableStream({ + start: (controller) => { + controller.enqueue(streamedResponse); + controller.close(); + }, + }) + ) + ); + }); + + for await (const { submitAndAwait } of provider.operations.submitAndAwait({ + encodedTransaction: "it's mocked so doesn't matter", + })) { + expect(submitAndAwait.type).toEqual('SuccessStatus'); + expect((submitAndAwait).time).toEqual('data: 4611686020137152060'); + } + }); + + it('should throw if the subscription stream data string parsing fails for some reason', async () => { + const provider = await Provider.create(FUEL_NETWORK_URL); + + const badResponse = 'data: whatever'; + vi.spyOn(global, 'fetch').mockImplementationOnce(() => { + const streamResponse = new TextEncoder().encode(badResponse); + return Promise.resolve( + new Response( + new ReadableStream({ + start: (controller) => { + controller.enqueue(streamResponse); + controller.close(); + }, + }) + ) + ); + }); + + await expectToThrowFuelError( + async () => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for await (const { submitAndAwait } of provider.operations.submitAndAwait({ + encodedTransaction: "it's mocked so doesn't matter", + })) { + // shouldn't be reached! + expect(true).toBeFalsy(); + } + }, + { + code: FuelError.CODES.STREAM_PARSING_ERROR, + message: `Error while parsing stream data response: ${badResponse}`, + } + ); + }); + test('requestMiddleware modifies the request before being sent to the node [sync]', async () => { const fetchSpy = vi.spyOn(global, 'fetch'); await Provider.create(FUEL_NETWORK_URL, { diff --git a/packages/errors/src/error-codes.ts b/packages/errors/src/error-codes.ts index 6c5e5b2838e..580e3da0e85 100644 --- a/packages/errors/src/error-codes.ts +++ b/packages/errors/src/error-codes.ts @@ -100,6 +100,9 @@ export enum ErrorCode { SCRIPT_REVERTED = 'script-reverted', SCRIPT_RETURN_INVALID_TYPE = 'script-return-invalid-type', + // graphql + STREAM_PARSING_ERROR = 'stream-parsing-error', + // coder // ... } diff --git a/packages/fuel-gauge/src/transaction-response.test.ts b/packages/fuel-gauge/src/transaction-response.test.ts index 2ce237d127f..ebc709c6b3c 100644 --- a/packages/fuel-gauge/src/transaction-response.test.ts +++ b/packages/fuel-gauge/src/transaction-response.test.ts @@ -277,5 +277,5 @@ describe('TransactionResponse', () => { { code: ErrorCode.TRANSACTION_SQUEEZED_OUT } ); cleanup(); - }); + }, 7000); });