Skip to content

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Feb 22, 2023

Previously the discovery handler would not gracefully handle an empty HELLO command:

$ ./dummy-discovery
HELLO
panic: runtime error: slice bounds out of range [6:5]

goroutine 1 [running]:
github.com/arduino/pluggable-discovery-protocol-handler/v2.(*Server).Run(0xc00002c1e0, {0x5147c8?, 0xc000012010?}, {0x5147e8?, 0xc000012018?})
        /home/megabug/Workspace/pluggable-discovery-protocol-handler/discovery.go:135 +0x5a5
main.main()
        /home/megabug/Workspace/pluggable-discovery-protocol-handler/dummy-discovery/main.go:44 +0x88
$

After this patch, it is handled correctly:

$ ./dummy-discovery 
HELLO
{
  "eventType": "hello",
  "message": "Invalid HELLO command",
  "error": true
}
QUIT
{
  "eventType": "quit",
  "message": "OK"
}
$

Fix #32

@cmaglie cmaglie self-assigned this Feb 22, 2023
@cmaglie cmaglie added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Feb 22, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2023

Codecov Report

Base: 0.00% // Head: 0.00% // No change to project coverage 👍

Coverage data is based on head (71b0ce2) compared to base (69bcf9a).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@          Coverage Diff          @@
##            main     #33   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          4       4           
  Lines        255     259    +4     
=====================================
- Misses       255     259    +4     
Flag Coverage Δ
unit 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
discovery.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Thanks Cristian!

@cmaglie cmaglie merged commit 64ac0f7 into main Feb 22, 2023
@cmaglie cmaglie deleted the fix_hello_handling branch February 22, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when no arguments passed via HELLO command
3 participants