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

new nodejs parser: llhttp #31

Closed
Skorpeo opened this issue Jan 21, 2019 · 9 comments
Closed

new nodejs parser: llhttp #31

Skorpeo opened this issue Jan 21, 2019 · 9 comments

Comments

@Skorpeo
Copy link

Skorpeo commented Jan 21, 2019

New node js parser: nodejs/http-parser#285 (comment)

@victoraugustolls
Copy link
Contributor

Hey @elprans ! Do you have an opinion about moving to llhttp? As http-parser is no longer maintained :(

@elprans
Copy link
Member

elprans commented Oct 13, 2020

We don't really have the bandwidth to make the switch, but if anyone wants to pick this up, patches are welcome!

@victoraugustolls
Copy link
Contributor

Hi @elprans ! I'm trying to take this one here: https://github.com/victoraugustolls/httptools

I'm having some trouble with the tests due to some missing exceptions between http-parser and llhttp. Could you help me? Or point me someone to help?

The problem is that there are no callback errors for on_body, on_header, on_url and on_status. Other point is that I had to change how the upgrade works, but don't know if this is correct.

Maybe @indutny can help also? (sorry if tagging you here is wrong)

@indutny
Copy link

indutny commented Oct 22, 2020

@victoraugustolls I'd be happy to help!

You basically need to do what Node.js does at this moment if you'd like to raise an error from the on_body/etc callbacks:

      llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
      return HPE_USER;

@victoraugustolls
Copy link
Contributor

Perfect! Thanks @indutny ! Will give it a go over the weekend!

@abersheeran
Copy link

Is there any progress?

@victoraugustolls
Copy link
Contributor

Hey @abersheeran ! There is an open PR for it: #56

@1st1
Copy link
Member

1st1 commented Dec 9, 2020

We've hit a delay with EdgeDB, but integrating httptools deeper in it is on my immediate todo. And so I'll be closely looking at that and will also likely release uvloop for 3.9. Sorry for the delay.

@fantix
Copy link
Member

fantix commented Mar 30, 2021

Thanks to @victoraugustolls and everyone, this is done in #56. I'll get a release out soon.

@fantix fantix closed this as completed Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants