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

Make submitAndAwait errors immediately identifiable #2108

Closed
nedsalk opened this issue Aug 20, 2024 · 1 comment · Fixed by #2120
Closed

Make submitAndAwait errors immediately identifiable #2108

nedsalk opened this issue Aug 20, 2024 · 1 comment · Fixed by #2120
Assignees
Labels
good first issue Good for newcomers

Comments

@nedsalk
Copy link

nedsalk commented Aug 20, 2024

I've been trying to implement support for the submitAndAwait subscription but came across a problem related to invalid transactions. The current workflow of the TS SDK is as follows:

  • Submit a transaction via the submit mutation
    • If the transaction is invalid, throw immediately
    • Else, continue

Because the submit endpoint is a mutation, its response is immediate - either it fails or succeeds in tx submission.

When implementing submitAndAwait, I wanted to have behavior similar to the one described above:

  • If the transaction is invalid, throw immediately
  • Else, return an async iterator which has a .next() method which the user can call to await the subscription whenever they see fit

The problem I encountered is that I can't know if a transaction validity error happened until I read the response body of the subscription, but the response isn't necessarily immediate because the transaction might not have failed and is awaiting execution, in which case me trying to read the body would hang until the status update is streamed, which will block us from returning the iterator to the user.

The problem in code looks like this:

/**
* Create a connection.
* This will be a 200 OK response that has no info itself on if the transaction failed.
* All the info is in the response body.
*/
const response = await fetchFn(`${url}-sub`, {
  method: 'POST',  
  body, 
  headers,
});

/**
* The body is a readable byte stream.
* We `tee` it to get two independent streams that have the same underlying source from the subscription, 
* so that we can read the error while retaining one stream that we return to the user at the end.
* Note that reading the body is an inherently asynchronous operation, 
* regardless if the request is an SSE or a "regular" request.
* The only difference is that the byte stream is immediately closed once read in regular requests, 
* but it remains open in SSE until the fuel-core node closes it.
*/
const [errorReader, resultReader] = response.body.tee().map((stream) => stream.getReader());

/**
* FuelGraphqlSubscriber is an iterator that is able to read subscriptions returned from our fuel-core nodes.
*/
const errorChecker = new FuelGraphqlSubscriber(errorReader);

/**
* We call the .next() method to check if there's an error.
* This will throw if the transaction is invalid - which is what we want,
* but it will also hang if the transaction was submitted until its status updates.
* The hanging is a problem because it blocks the user 
* and defeats the purpose of the async iterator returned at the end.
*/
await errorChecker.next();

return new FuelGraphqlSubscriber(resultReader);

After pondering about it, two solutions came to mind:

  1. If the transaction fails, add some response header indicating that it failed so that we can do an if/else immediately
if(response.headers.has('transaction-failed') {
  // will certainly throw immediately
  await errorChecker.next();
}
  1. Instead of awaiting the commit and not returning anything until the status changes, return a stream as is done for statusChange . This would immediately return a SubmittedStatus response, which is great because we would then know that we'll get a response immediately from the node and can check if it's an error or not.
// will resolve immediately
// either by throwing or reading the `SubmittedStatus` which we'll just discard
await errorChecker.next(); 

I am more inclined to the second solution because the SubmittedStatus also contains the time of submission which may be of interest for some.

@xgreenx
Copy link
Collaborator

xgreenx commented Aug 20, 2024

I think we could add a new endpoint - submit_and_await_status. Which will also return the SubmittedStatus status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants