frontend: move cable-guy API calls to ethernet store#3620
Merged
patrickelectric merged 1 commit intobluerobotics:masterfrom Oct 24, 2025
Merged
frontend: move cable-guy API calls to ethernet store#3620patrickelectric merged 1 commit intobluerobotics:masterfrom
patrickelectric merged 1 commit intobluerobotics:masterfrom
Conversation
Reviewer's GuideMoves cable-guy API calls from components into the Ethernet Vuex store, replacing direct back_axios usage with typed store actions and consolidating notification handling. Sequence diagram for component API call delegation to EthernetStoresequenceDiagram
participant C as "Component (e.g. InterfaceCard)"
participant S as "EthernetStore"
participant A as "back_axios"
participant N as "Notifier"
C->>S: call deleteAddress({interface_name, ip_address})
S->>A: send DELETE /address
A-->>S: response or error
alt success
S-->>C: resolve
else error
S->>N: pushError('ETHERNET_ADDRESS_DELETE_FAIL', error)
S-->>C: throw error
end
Sequence diagram for updating host DNS via EthernetStoresequenceDiagram
participant C as "Component (DnsConfigurationMenu)"
participant S as "EthernetStore"
participant A as "back_axios"
participant N as "Notifier"
C->>S: call updateHostDNS({host_nameservers, is_locked})
S->>A: POST /host_dns
A-->>S: response or error
alt success
S->>N: pushSuccess('APPLY_HOST_DNS_SUCCESS', ...)
S-->>C: resolve
else error
S->>N: pushError('APPLY_HOST_DNS_FAIL', error)
S-->>C: throw error
end
Class diagram for updated EthernetStore actionsclassDiagram
class EthernetStore {
+addAddress(payload)
+deleteAddress(payload)
+addDHCPServer(payload)
+RemoveDHCPServer(interface_name)
+getDHCPServerDetails(interface_name)
+getHostDNS()
+updateHostDNS(payload)
+getAvailableInterfaces()
+setInterfacesPriority(interfaces)
+triggerDynamicIP(interface_name)
}
EthernetStore --> Notifier : uses
EthernetStore --> back_axios : uses
Notifier : pushBackError()
Notifier : pushError()
Notifier : pushSuccess()
back_axios : axios instance
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `core/frontend/src/store/ethernet.ts:42-51` </location>
<code_context>
this.updating_host_nameservers = true
this.error = null
- await back_axios({
- method: 'get',
- url: `${ethernet.API_URL}/host_dns`,
- timeout: 15000,
- })
+ await ethernet.getHostDNS()
.then((response) => {
const { data } = response
this.host_nameservers = data.nameservers as string[]
this.is_locked = data.lock as boolean
})
- .catch((error) => {
- ethernet.setInterfaces([])
- notifier.pushBackError('HOST_DNS_FETCH_FAIL', error)
</code_context>
<issue_to_address>
**suggestion:** Error handling in RemoveDHCPServer does not rethrow the error, unlike other actions.
Please rethrow the error after notifying, as in other actions, to ensure consistent and predictable error handling.
</issue_to_address>
### Comment 2
<location> `core/frontend/src/store/ethernet.ts:115-122` </location>
<code_context>
+ }
+
+ @Action
+ async getDHCPServerDetails(interface_name: string): Promise<DHCPServerDetails> {
+ return await back_axios({
+ method: 'get',
</code_context>
<issue_to_address>
**suggestion (bug_risk):** getDHCPServerDetails does not handle errors, which may lead to unhandled promise rejections.
Please add error handling to this method to prevent unhandled promise rejections.
```suggestion
@Action
async getDHCPServerDetails(interface_name: string): Promise<DHCPServerDetails | null> {
try {
return await back_axios({
method: 'get',
url: `${this.API_URL}/dhcp/details/${interface_name}`,
timeout: 15000,
})
} catch (error: any) {
const message = `Could not fetch DHCP server details for interface '${interface_name}': ${error.message}.`
notifier.pushError('DHCP_SERVER_DETAILS_FAIL', message)
return null
}
}
```
</issue_to_address>
### Comment 3
<location> `core/frontend/src/store/ethernet.ts:125` </location>
<code_context>
+ }
+
+ @Action
+ async getHostDNS() {
+ return await back_axios({
+ method: 'get',
</code_context>
<issue_to_address>
**suggestion:** getHostDNS returns the raw axios promise, which may not match the expected return type.
Consider returning only the necessary data from the axios response to ensure the method's return type matches expectations and to prevent exposing internal response details.
Suggested implementation:
```typescript
@Action
async getHostDNS(): Promise<any> {
const { data } = await back_axios({
method: 'get',
url: `${this.API_URL}/host_dns`,
});
return data;
```
- Replace `any` in the return type with a more specific type if you know the expected shape of the DNS data.
- If you have a DNS details interface (e.g., `HostDNSDetails`), use `Promise<HostDNSDetails>` instead of `Promise<any>`.
</issue_to_address>
### Comment 4
<location> `core/frontend/src/components/ethernet/InterfaceCard.vue:323` </location>
<code_context>
- .catch((error) => {
- notifier.pushBackError('ETHERNET_ADDRESS_DELETE_FAIL', error)
- })
+ await ethernet.deleteAddress({ interface_name: this.adapter.name, ip_address: ip })
},
async triggerForDynamicIP(): Promise<void> {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** deleteAddress action is awaited but errors are not handled locally.
Handle errors from deleteAddress locally to avoid unhandled promise rejections and enable user feedback.
```suggestion
try {
await ethernet.deleteAddress({ interface_name: this.adapter.name, ip_address: ip })
} catch (error) {
notifier.pushBackError('ETHERNET_ADDRESS_DELETE_FAIL', error)
}
```
</issue_to_address>
### Comment 5
<location> `core/frontend/src/components/ethernet/InterfaceCard.vue:336` </location>
<code_context>
- const message = `Could not remove DHCP server from interface '${this.adapter.name}': ${error.message}.`
- notifier.pushError('DHCP_SERVER_REMOVE_FAIL', message)
- })
+ await ethernet.RemoveDHCPServer(this.adapter.name)
},
async fetchLeases(): Promise<void> {
</code_context>
<issue_to_address>
**suggestion:** RemoveDHCPServer is called with PascalCase, which is inconsistent with other method calls.
Consider renaming the method to 'removeDHCPServer' to match the naming convention used elsewhere.
Suggested implementation:
```
await ethernet.removeDHCPServer(this.adapter.name)
```
If the `ethernet` module currently exports the method as `RemoveDHCPServer`, you will also need to rename the method in that module to `removeDHCPServer` and update any other references to it.
</issue_to_address>
### Comment 6
<location> `core/frontend/src/components/app/NetworkInterfacePriorityMenu.vue:141-145` </location>
<code_context>
- const message = `Could not set network interface priorities: ${interface_priorities}, error: ${error}`
- notifier.pushError('INCREASE_NETWORK_INTERFACE_METRIC_FAIL', message)
- })
+ await ethernet.setInterfacesPriority(interface_priorities)
.then(() => {
- notifier.pushSuccess(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** setInterfacesPriority is awaited and errors are not handled locally.
Handle errors within this function to avoid unhandled promise rejections and improve user feedback.
```suggestion
try {
await ethernet.setInterfacesPriority(interface_priorities)
this.close()
} catch (error) {
const message = `Could not set network interface priorities: ${interface_priorities}, error: ${error}`
notifier.pushError('INCREASE_NETWORK_INTERFACE_METRIC_FAIL', message)
}
await this.fetchAvailableInterfaces()
```
</issue_to_address>
### Comment 7
<location> `core/frontend/src/components/app/NetworkInterfacePriorityMenu.vue:155-158` </location>
<code_context>
- // Necessary since the system can hang with dhclient timeouts
- timeout: 10000,
- })
+ await ethernet.getAvailableInterfaces()
.then((response) => {
const interfaces = response.data as EthernetInterface[]
</code_context>
<issue_to_address>
**suggestion (bug_risk):** getAvailableInterfaces is awaited and errors are not handled locally.
Handle errors within this function to avoid unhandled promise rejections and improve user feedback.
```suggestion
async fetchAvailableInterfaces(): Promise<void> {
try {
const response = await ethernet.getAvailableInterfaces()
const interfaces = response.data as EthernetInterface[]
interfaces.sort((a, b) => {
})
this.interfaces = interfaces
} catch (error) {
// Handle error locally, e.g. log or set an error state
console.error('Failed to fetch available interfaces:', error)
// Optionally, you could set an error property on this component for user feedback
// this.fetchInterfacesError = error
}
},
```
</issue_to_address>
### Comment 8
<location> `core/frontend/src/components/ethernet/AddressCreationDialog.vue:87` </location>
<code_context>
- ip_address: this.ip_address,
- },
- })
+ await ethernet.addAddress({ interface_name: this.interfaceName, ip_address: this.ip_address })
.then(() => {
this.form.reset()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** addAddress is awaited and errors are not handled locally.
Handle errors within this block to ensure users receive feedback and to avoid unhandled promise rejections.
Suggested implementation:
```
try {
await ethernet.addAddress({ interface_name: this.interfaceName, ip_address: this.ip_address })
this.form.reset()
return true
} catch (error) {
// Provide user feedback, e.g. set an error message or show a notification
this.errorMessage = error?.message || 'Failed to add address'
// Optionally, log the error
console.error('Error adding address:', error)
return false
}
```
- Ensure that `this.errorMessage` is defined in your component's data if you use it for feedback.
- If you use a notification system (e.g., `this.$notify`), replace the error feedback line accordingly.
</issue_to_address>
### Comment 9
<location> `core/frontend/src/components/ethernet/EthernetUpdater.vue:23` </location>
<code_context>
- // Necessary since the system can hang with dhclient timeouts
- timeout: 10000,
- })
+ await ethernet.getAvailableInterfaces()
.then((response) => {
const interfaces = response.data as EthernetInterface[]
</code_context>
<issue_to_address>
**suggestion (bug_risk):** getAvailableInterfaces is awaited and errors are not handled locally.
Handle errors within this function to avoid unhandled promise rejections and improve user feedback.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+42
to
+51
| await back_axios({ | ||
| method: 'post', | ||
| url: `${this.API_URL}/address`, | ||
| timeout: 10000, | ||
| params: { | ||
| interface_name: payload.interface_name, | ||
| ip_address: payload.ip_address, | ||
| }, | ||
| }) | ||
| .catch((error) => { |
There was a problem hiding this comment.
suggestion: Error handling in RemoveDHCPServer does not rethrow the error, unlike other actions.
Please rethrow the error after notifying, as in other actions, to ensure consistent and predictable error handling.
Comment on lines
+115
to
+122
| @Action | ||
| async getDHCPServerDetails(interface_name: string): Promise<DHCPServerDetails> { | ||
| return await back_axios({ | ||
| method: 'get', | ||
| url: `${this.API_URL}/dhcp/details/${interface_name}`, | ||
| timeout: 15000, | ||
| }) | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): getDHCPServerDetails does not handle errors, which may lead to unhandled promise rejections.
Please add error handling to this method to prevent unhandled promise rejections.
Suggested change
| @Action | |
| async getDHCPServerDetails(interface_name: string): Promise<DHCPServerDetails> { | |
| return await back_axios({ | |
| method: 'get', | |
| url: `${this.API_URL}/dhcp/details/${interface_name}`, | |
| timeout: 15000, | |
| }) | |
| } | |
| @Action | |
| async getDHCPServerDetails(interface_name: string): Promise<DHCPServerDetails | null> { | |
| try { | |
| return await back_axios({ | |
| method: 'get', | |
| url: `${this.API_URL}/dhcp/details/${interface_name}`, | |
| timeout: 15000, | |
| }) | |
| } catch (error: any) { | |
| const message = `Could not fetch DHCP server details for interface '${interface_name}': ${error.message}.` | |
| notifier.pushError('DHCP_SERVER_DETAILS_FAIL', message) | |
| return null | |
| } | |
| } |
| } | ||
|
|
||
| @Action | ||
| async getHostDNS() { |
There was a problem hiding this comment.
suggestion: getHostDNS returns the raw axios promise, which may not match the expected return type.
Consider returning only the necessary data from the axios response to ensure the method's return type matches expectations and to prevent exposing internal response details.
Suggested implementation:
@Action
async getHostDNS(): Promise<any> {
const { data } = await back_axios({
method: 'get',
url: `${this.API_URL}/host_dns`,
});
return data;- Replace
anyin the return type with a more specific type if you know the expected shape of the DNS data. - If you have a DNS details interface (e.g.,
HostDNSDetails), usePromise<HostDNSDetails>instead ofPromise<any>.
| .catch((error) => { | ||
| notifier.pushBackError('ETHERNET_ADDRESS_DELETE_FAIL', error) | ||
| }) | ||
| await ethernet.deleteAddress({ interface_name: this.adapter.name, ip_address: ip }) |
There was a problem hiding this comment.
suggestion (bug_risk): deleteAddress action is awaited but errors are not handled locally.
Handle errors from deleteAddress locally to avoid unhandled promise rejections and enable user feedback.
Suggested change
| await ethernet.deleteAddress({ interface_name: this.adapter.name, ip_address: ip }) | |
| try { | |
| await ethernet.deleteAddress({ interface_name: this.adapter.name, ip_address: ip }) | |
| } catch (error) { | |
| notifier.pushBackError('ETHERNET_ADDRESS_DELETE_FAIL', error) | |
| } |
| const message = `Could not remove DHCP server from interface '${this.adapter.name}': ${error.message}.` | ||
| notifier.pushError('DHCP_SERVER_REMOVE_FAIL', message) | ||
| }) | ||
| await ethernet.RemoveDHCPServer(this.adapter.name) |
There was a problem hiding this comment.
suggestion: RemoveDHCPServer is called with PascalCase, which is inconsistent with other method calls.
Consider renaming the method to 'removeDHCPServer' to match the naming convention used elsewhere.
Suggested implementation:
await ethernet.removeDHCPServer(this.adapter.name)
If the ethernet module currently exports the method as RemoveDHCPServer, you will also need to rename the method in that module to removeDHCPServer and update any other references to it.
Comment on lines
+141
to
145
| await ethernet.setInterfacesPriority(interface_priorities) | ||
| .then(() => { | ||
| notifier.pushSuccess( | ||
| 'INCREASE_NETWORK_INTERFACE_METRIC_SUCCESS', | ||
| 'Network interface priorities successfully updated!', | ||
| true, | ||
| ) | ||
| this.close() | ||
| }) | ||
| await this.fetchAvailableInterfaces() |
There was a problem hiding this comment.
suggestion (bug_risk): setInterfacesPriority is awaited and errors are not handled locally.
Handle errors within this function to avoid unhandled promise rejections and improve user feedback.
Suggested change
| await ethernet.setInterfacesPriority(interface_priorities) | |
| .then(() => { | |
| notifier.pushSuccess( | |
| 'INCREASE_NETWORK_INTERFACE_METRIC_SUCCESS', | |
| 'Network interface priorities successfully updated!', | |
| true, | |
| ) | |
| this.close() | |
| }) | |
| await this.fetchAvailableInterfaces() | |
| try { | |
| await ethernet.setInterfacesPriority(interface_priorities) | |
| this.close() | |
| } catch (error) { | |
| const message = `Could not set network interface priorities: ${interface_priorities}, error: ${error}` | |
| notifier.pushError('INCREASE_NETWORK_INTERFACE_METRIC_FAIL', message) | |
| } | |
| await this.fetchAvailableInterfaces() |
Comment on lines
155
to
158
| }) | ||
| this.interfaces = interfaces | ||
| }) | ||
| .catch((error) => { | ||
| ethernet.setInterfaces([]) | ||
| notifier.pushBackError('ETHERNET_AVAILABLE_INTERFACES_FETCH_FAIL', error) | ||
| }) | ||
| }, |
There was a problem hiding this comment.
suggestion (bug_risk): getAvailableInterfaces is awaited and errors are not handled locally.
Handle errors within this function to avoid unhandled promise rejections and improve user feedback.
Suggested change
| }) | |
| this.interfaces = interfaces | |
| }) | |
| .catch((error) => { | |
| ethernet.setInterfaces([]) | |
| notifier.pushBackError('ETHERNET_AVAILABLE_INTERFACES_FETCH_FAIL', error) | |
| }) | |
| }, | |
| async fetchAvailableInterfaces(): Promise<void> { | |
| try { | |
| const response = await ethernet.getAvailableInterfaces() | |
| const interfaces = response.data as EthernetInterface[] | |
| interfaces.sort((a, b) => { | |
| }) | |
| this.interfaces = interfaces | |
| } catch (error) { | |
| // Handle error locally, e.g. log or set an error state | |
| console.error('Failed to fetch available interfaces:', error) | |
| // Optionally, you could set an error property on this component for user feedback | |
| // this.fetchInterfacesError = error | |
| } | |
| }, |
| // Necessary since the system can hang with dhclient timeouts | ||
| timeout: 10000, | ||
| }) | ||
| await ethernet.getAvailableInterfaces() |
There was a problem hiding this comment.
suggestion (bug_risk): getAvailableInterfaces is awaited and errors are not handled locally.
Handle errors within this function to avoid unhandled promise rejections and improve user feedback.
patrickelectric
approved these changes
Oct 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix: #1841
tested all endpoints affected
Summary by Sourcery
Centralize Ethernet-related API calls into the Vuex ethernet store and remove direct axios usage from components
Enhancements: