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

Improve handling of ris-live HTTP errors #155

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pforemski
Copy link

Hi,

While debugging ris-live errors (HTTP ERROR: Transferred a partial file (18)) I found a few small issues in the code, fixed/improved in the below 2 commits.

  • fc71272 fixes an integer overflow error, where a timestamp in milliseconds was written to an uint32_t, which somehow didn't manifest for me
  • 44fb56e is a poor-man's solution to no HTTP read error handling in ris-live (I was really surprised to see this guys... :P) see also Handle ris-live stream error messages #139

Thanks for your great work!

@alistairking
Copy link
Member

Hey!
Thanks for taking the time to look into this, and for the PR.

The integer overflow bug is a nice catch. Thanks.

I'm not too sure about handling the read error like that, so I'll have to spend some quality time with the code to double check that it doesn't have unwanted side effects (it's probably ok, but since that code is right in the core of bgpstream, i just want to be extra careful).

I do think however, that when we get EOF from the transport (the 0 return), then we should return END_OF_DUMP. Remember that we don't know what transport we are reading the data over, so it is perfectly reasonable to be reading ris-live formatted data from a file (indeed this is how we test things).

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

Successfully merging this pull request may close these issues.

None yet

2 participants