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

return if no vrps are loaded #68

Merged
merged 1 commit into from
Jul 11, 2022
Merged

return if no vrps are loaded #68

merged 1 commit into from
Jul 11, 2022

Conversation

job
Copy link
Member

@job job commented May 22, 2022

If the VRP set can't be retrieved, an error pops up:

feather$ ./stayrtr -cache http://localhost/
ERRO[0000] Error updating: Get "http://localhost/": dial tcp [::1]:80: connect: connection refused
WARN[0000] Error setting up intial state: parsing time "" as "2006-01-02T15:04:05Z07:00": cannot parse "" as "2006"
WARN[0000] Initial sync not complete. Refreshing every 30 seconds
INFO[0000] StayRTR Server started (sessionID:44976, refresh:3600, retry:600, expire:7200)

This PR silences the error by returning before the checkTime/buildtime dance - but I'm not sure if that's the best path forward.

@job job requested a review from ties May 22, 2022 16:06
@ties
Copy link
Collaborator

ties commented May 23, 2022

Do you think https://datatracker.ietf.org/doc/html/rfc6810#section-6.4 is well handled in routers? (And when I'm on a desktop pc I'll check if we return that PDU)

I will look at this PR better later. No vpr attribute in the json (not even an empty list) feels like an error state, maybe we should return an error about unexpected data?

@@ -255,6 +255,11 @@ func (e IdenticalFile) Error() string {
func (s *state) updateFromNewState() error {
sessid := s.server.GetSessionId()

vrpsjson := s.lastdata.Data
if (vrpsjson == nil) {
return nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that we should probably return an error here. I need to look at the complete structure of the object we have here though.

@ties
Copy link
Collaborator

ties commented Jul 11, 2022

Definitely a local improvement 👍

@ties ties merged commit 68ca076 into master Jul 11, 2022
@ties ties deleted the bugfix branch July 11, 2022 09:46
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.

2 participants