Skip to content

RSDK-12499: better error log when host is offline #505

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.

Sign up for GitHub

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
+6 −5
Open

RSDK-12499: better error log when host is offline #505

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cec05c7
change logger
allisonschiang Oct 13, 2025
13c87f2
Delete panic_test.go
allisonschiang Oct 16, 2025
a189af2
warning remove
allisonschiang Nov 11, 2025
4cd064c
Revert "change logger"
allisonschiang Nov 11, 2025
e1acc46
j
allisonschiang Nov 12, 2025
6410152
Merge branch 'viamrobotics:main' into RSDK-12499
allisonschiang Nov 12, 2025
a91b8a2
Revert "j"
allisonschiang Nov 12, 2025
ff27d9d
Revert "warning remove"
allisonschiang Nov 12, 2025
3e9cabe
lower to debug
allisonschiang Nov 12, 2025
054142e
Update wrtc_signaling_answerer.go
allisonschiang Nov 12, 2025
def83ea
Revert "Update wrtc_signaling_answerer.go"
allisonschiang Nov 12, 2025
b916a0c
Update wrtc_call_queue_mongodb.go
allisonschiang Nov 14, 2025
8f57c4a
Update wrtc_call_queue_mongodb.go
allisonschiang Nov 14, 2025
359b70b
Update wrtc_call_queue_mongodb.go
allisonschiang Nov 14, 2025
ec32962
Update wrtc_call_queue_mongodb.go
allisonschiang Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions rpc/wrtc_call_queue_mongodb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ func (queue *mongoDBWebRTCCallQueue) checkHostQueueSize(ctx context.Context, for
return errTooManyConns
}

var errOffline = status.Error(codes.Unavailable, "host appears to be offline; ensure machine is online and try again")
var ErrOffline = errors.New("host appears to be offline; ensure machine is online and try again")
Copy link
Member

@benjirewis benjirewis Nov 14, 2025

Choose a reason for hiding this comment

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

Why remove the "unavailable" status?

Copy link
Member

@benjirewis benjirewis Nov 14, 2025

Choose a reason for hiding this comment

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

Ah I see the comment from your other PR.

Copy link
Contributor Author

@allisonschiang allisonschiang Nov 14, 2025

Choose a reason for hiding this comment

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

updated it to just check for ErrOffline by making ErrOffline a regular error instead of a status. I think making it status() was double wrapping it down the line because the original error was rpc error: code = Unavailable desc = requestID=99cc1420b3f4e658458dc61ec70cb033: rpc error: code = Unavailable desc = host appears to be offline; ensure machine is online and try again; context deadline exceeded; mDNS query failed to find a candidate

pasting it here so its easier for others to see^!

Copy link
Member

@cheukt cheukt Nov 14, 2025

Choose a reason for hiding this comment

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

what does the error become with this change? you can test this by pointing goutils on app to this change and running the cli/sdk against it. I ask because I would like to keep the Unavailable code if possible, so if we lose that we should think of ways to keep it in

Copy link
Contributor Author

@allisonschiang allisonschiang Nov 17, 2025

Choose a reason for hiding this comment

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

I'm reverting this change because the new error status has the error as "unknown" vs "unavailable". So something else is re-wrapping this error status but without the "unavailable" status wanted. I think you can get the error message from the status by converting it later on anyways


// checkHostOnline will check if there is some operator for all the managed hosts that
// claims to have an answerer online for that host. It does this by running an aggregation
Expand Down Expand Up @@ -1128,7 +1128,7 @@ func (queue *mongoDBWebRTCCallQueue) checkHostOnline(ctx context.Context, hosts
return err
}
if len(ret) == 0 {
return errOffline
return ErrOffline
}
return nil
}
Expand All @@ -1149,7 +1149,7 @@ func (queue *mongoDBWebRTCCallQueue) incrementConnectionEstablishmentExpectedFai
// error (internal error from MDB query, e.g.). We can tell from the passed in error.

reason := "other"
if errors.Is(err, errOffline) {
if errors.Is(err, ErrOffline) {
reason = "answerers_offline"
} else if errors.Is(err, errTooManyConns) {
reason = "too_many_callers"
Expand Down
4 changes: 2 additions & 2 deletions rpc/wrtc_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func dialWebRTC(

utils.PanicCapturingGo(func() {
if err := exchangeCandidates(); err != nil {
logger.Warnw("Failed to exchange candidates", "err", err)
logger.Debugw("Failed to exchange candidates", "err", err)
exchangeCancel(err)
}
})
Expand All @@ -411,7 +411,7 @@ func dialWebRTC(
Error: ErrorToStatus(exchangeErr).Proto(),
},
}); err != nil {
logger.Warnw("Problem sending error to signaling server", "err", err)
logger.Debugw("Problem sending error to signaling server", "err", err)
}
})
return nil, exchangeErr
Expand Down
Loading