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

Stellar SDK doesn't fully support HTTP2 #612

Open
doasync opened this issue Jan 18, 2021 · 8 comments
Open

Stellar SDK doesn't fully support HTTP2 #612

doasync opened this issue Jan 18, 2021 · 8 comments
Labels

Comments

@doasync
Copy link

doasync commented Jan 18, 2021

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:

if (error.response && error.response.status && error.response.statusText) {

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).

@doasync doasync added the bug label Jan 18, 2021
@Shaptic
Copy link
Contributor

Shaptic commented Jan 27, 2021

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.

@Fomin2402
Copy link

any news?

@Fomin2402
Copy link

Any news?

@Shaptic
Copy link
Contributor

Shaptic commented Jan 11, 2022

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?

@Fomin2402
Copy link

Fomin2402 commented Jan 12, 2022

@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)

@Fomin2402
Copy link

@Shaptic any news?

1 similar comment
@Fomin2402
Copy link

@Shaptic any news?

@Shaptic
Copy link
Contributor

Shaptic commented Jan 10, 2024

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();

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

No branches or pull requests

3 participants