rpc-error with severity warning should not be treated as errors

[steiler]

Hi @carlmontanari, a user of kubenet -> https://github.com/sdcio/data-server/issues/158 experienced the following issue with a Juniper device.

On the EditConfig call, the Junos would reply with an rpc-error with a severityof warning, indicating that the config was excepted but a license would be required, see:

rpc error: code = Unknown desc = operation error from input '<?xml version="1.0" encoding="UTF-8"?><rpc xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="581"><commit/></rpc>'. matched error sub-string '
<rpc-error>
  <error-severity>warning</error-severity>
  <source-daemon>
  mgd
  </source-daemon>
  <error-path>
  [edit routing-instances VRF-Example protocols]
  </error-path>
  <error-info>
    <bad-element>
    bgp
    </bad-element>
  </error-info>
  <error-message>
  requires 'bgp' license
  </error-message>
</rpc-error>

So in my view the error pattern https://github.com/scrapli/scrapligo/blob/main/response/netconf.go#L45 should be redefined to e.g. (?s)<rpc-errors?>(.*<error-severity>error</error-severity>.*)</rpc-errors?>, such that a fullblown error is ownly thrown on severity error but not on warnings.

Further allowing for a structured access to the warning would be beneficial, but could be done in sdcio via the raw data.

Whats your opinion on the whole story?

[carlmontanari]

well... it looks like I had at least the start of a thought of recording errors a little better from this 🤣 but looks like this is unused everywhere... so I can only conclude that I had great intentions!

I don't think I would exclude an error if its just a warning, but I for sure think we can store those in the ErrorMessages thing that does nothing right now... then if/when you have a failed response object you could check the error severities/messages to see if you think its truly failed, or if you just wanna ignore the error. Of course you could already do this by just checking the result yourself and then deciding if you want to go on or not, but yeah we can/should make it better so the response has something like this maybe?

type NetconfResponse struct {
<snip>
    WarningErrorMessages      []string
        ErrorMessages []string
<snip>
}

Maybe we could have some kind of global config too that controls if you are ok with "warnings" not causing a response to be "failed" too?

[steiler]

Just realized that not scraplygo is throwing an error but the external code does if r.failed contains content. So no need to have global config. I'll then simply rely on the length of ErrorMessages and WarningErrorMessage.