search

stellar / js-stellar-sdk

Main Stellar client library for the JavaScript language.
https://stellar.github.io/js-stellar-sdk/
Apache License 2.0
643 stars 314 forks source link

Stellar SDK doesn't fully support HTTP2 #612

Open doasync opened 3 years ago

doasync commented 3 years ago

Stellar SDK throws an error which is an instance of Error for 404 responses with HTTP2.

Version: stellar-sdk 7.0.0

To Reproduce

Try to load an account, which does not exist.

Expected behavior

Stellar SDK should throw NotFoundError for HTTP2 404 responses.

Additional context

Status-line in HTTP 1.1 consists of HTTP-Version Status-Code Reason-Phrase(rfc2616 spec here)

BUT in HTTP2 Reason-Phrase is deprecated (rfc7540 spec here):

HTTP/2 does not define a way to carry the version or reason phrase that is included in an HTTP/1.1 status line.

JS Stellar-SDK needs status text (reason phrase) to return a correct error object. It creates an error of the NotFoundError class only if the status text is present. See the code: https://github.com/stellar/js-stellar-sdk/blob/84d10c604e4f0217c608d4019ca3e80e35584eb5/src/call_builder.ts#L382

We later check if an error is an instance of NotFoundError. Here is an example of this check in the docs: https://developers.stellar.org/docs/tutorials/send-and-receive-payments/#send-a-payment

if (error instanceof StellarSdk.NotFoundError) { ... }

There is no status text in the HTTP2 response, and we cannot properly determine what kind of error occurred (parsing an error message is not a good idea).

Shaptic commented 3 years ago

Unfortunately, the HTTP client we use under the hood doesn't support HTTP2 yet (see axios/axios#1175). However, there may be a higher-level workaround for this. I'll be looking closer at error-handling in a broader sense in the coming weeks so this may get lumped into it.

Famin42 commented 3 years ago

any news?

Famin42 commented 2 years ago

Any news?

Shaptic commented 2 years ago

Hey @Fomin2402, thanks for following up. Unfortunately, this has gotten de-prioritized for some time in lieu of other projects (AMMs, muxed accounts, scaling, etc. to name a few). Is the lack of HTTP/2 support a major blocker for you?

Famin42 commented 2 years ago

@Shaptic Hi, I think it's pretty important, cause there are many servers which is under HTTP/2 protocol. For example: some proxies which is under HTTP/2 protocol will break your error handling logic (cause it based on statusText field, which is absent on HTTP/2)

Famin42 commented 2 years ago

@Shaptic any news?

Famin42 commented 2 years ago

@Shaptic any news?

Shaptic commented 10 months ago

Have you experimented with the HTTP/2 adapter? I haven't tried it yet myself, but it should be possible to override the default adapter like so:

import { Horizon } from '@stellar/stellar-sdk';
import { createHTTP2Adapter } from 'axios-http2-adapter';

Horizon.AxiosClient.defaults.adapter = createHTTP2Adapter();
janewang commented 1 month ago

Since we are removing axios dependency, could we unblock this ticket?