-
Notifications
You must be signed in to change notification settings - Fork 1
feat: websocket server poc #13
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
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
1d286b8
feat: naive websocket server
moreirathomas afeffb9
feat: single socket with dynamic event handling
moreirathomas 2a640fe
chore: rename command to ws (work in progress)
moreirathomas f23c073
feat: use websocket to access runner api
moreirathomas 286ed89
docs: write implem documentation
moreirathomas fd6f6c7
refactor: interface websocket io away from server
moreirathomas f12ec2f
chore: rename progress method to match standard
moreirathomas e86d39b
fix: data race on call to run.flush
moreirathomas 9d360c8
feat: send json message to client
moreirathomas 3151e2e
feat: accept json message from client
moreirathomas 109486b
refactor: remove output state and auto send output
moreirathomas 888abbc
refactor: typed message structures
moreirathomas 3bfc899
misc: rename structs
moreirathomas 5827cd4
refactor: move state around struct
moreirathomas ce6af4f
chore: rename member srv to service
moreirathomas d5444b1
fix: close the connection before flushing
moreirathomas 3b1a0e2
docs: Handler struct
moreirathomas 84dbced
refactor: merge Reader and Writer interface and rename package
moreirathomas 66fc8aa
refactor: extract upgrader
moreirathomas 60d6b94
docs: service.cancelRun panics
moreirathomas 88ce514
chore: smoother error handling
moreirathomas 850a1be
docs: document token
moreirathomas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| package websocketio | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "log" | ||
|
|
||
| "github.com/gorilla/websocket" | ||
| ) | ||
|
|
||
| type Reader interface { | ||
| ReadTextMessage() (string, error) | ||
| ReadJSON(v interface{}) error | ||
| } | ||
|
|
||
| type Writer interface { | ||
| WriteTextMessage(m string) error | ||
| WriteJSON(v interface{}) error | ||
| } | ||
|
|
||
| type ReadWriter interface { | ||
| Reader | ||
| Writer | ||
| } | ||
|
|
||
| type readWriter struct { | ||
| ws *websocket.Conn | ||
| silent bool | ||
| } | ||
|
|
||
| // NewReadWriter returns a concrete type ReadWriter | ||
| // reading from and writing to the websocket connection. | ||
| func NewReadWriter(ws *websocket.Conn, silent bool) ReadWriter { | ||
| return &readWriter{ws, silent} | ||
| } | ||
|
|
||
| func (rw *readWriter) ReadTextMessage() (string, error) { | ||
| messageType, p, err := rw.ws.ReadMessage() | ||
| if err != nil { | ||
| return "", fmt.Errorf("cannot read message: %s", err) | ||
| } | ||
|
|
||
| if messageType != websocket.TextMessage { | ||
| return "", fmt.Errorf("message type is not TextMessage") | ||
| } | ||
|
|
||
| m := string(p) | ||
|
|
||
| if !rw.silent { | ||
| log.Printf("<- %s", m) | ||
| } | ||
|
|
||
| return m, nil | ||
| } | ||
|
|
||
| func (rw *readWriter) ReadJSON(v interface{}) error { | ||
| err := rw.ws.ReadJSON(v) | ||
| if err != nil { | ||
| return fmt.Errorf("cannot read message: %s", err) | ||
| } | ||
|
|
||
| if !rw.silent { | ||
| log.Printf("<- %v", v) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (rw *readWriter) WriteTextMessage(m string) error { | ||
| err := rw.ws.WriteMessage(websocket.TextMessage, []byte(m)) | ||
| if err != nil { | ||
| return fmt.Errorf("cannot write message: %s", err) | ||
| } | ||
|
|
||
| if !rw.silent { | ||
| log.Printf("-> %s", m) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (rw *readWriter) WriteJSON(v interface{}) error { | ||
| err := rw.ws.WriteJSON(v) | ||
| if err != nil { | ||
| return fmt.Errorf("cannot write message: %s", err) | ||
| } | ||
|
|
||
| if !rw.silent { | ||
| log.Printf("-> %v", v) | ||
| } | ||
|
|
||
| return nil | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "log" | ||
| "net/http" | ||
|
|
||
| "github.com/benchttp/engine/internal/configparse" | ||
| "github.com/benchttp/engine/internal/websocketio" | ||
| "github.com/benchttp/engine/runner" | ||
| ) | ||
|
|
||
| // Handler implements http.Handler. | ||
| // It serves a websocket server allowing | ||
| // remote manipulation of runner.Runner. | ||
| type Handler struct { | ||
| Silent bool | ||
| Token string | ||
| service *service | ||
| } | ||
|
|
||
| func NewHandler(silent bool, token string) *Handler { | ||
| return &Handler{ | ||
| Silent: silent, | ||
| Token: token, | ||
| service: &service{}, | ||
| } | ||
| } | ||
|
|
||
| func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
| switch r.URL.Path { | ||
| case "/run": | ||
| h.handle(w, r) | ||
| default: | ||
| http.NotFound(w, r) | ||
| } | ||
| } | ||
|
|
||
| func (h *Handler) handle(w http.ResponseWriter, r *http.Request) { | ||
| upgrader := secureUpgrader(h.Token) | ||
| ws, err := upgrader.Upgrade(w, r, nil) | ||
| if err != nil { | ||
| http.Error(w, err.Error(), http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| defer func() { | ||
| ws.Close() | ||
| // The client is gone, flush all the state. | ||
| // TODO Handle reconnect? | ||
| h.service.flush() | ||
| }() | ||
|
|
||
| log.Println("connected with client via websocket") | ||
|
|
||
| rw := websocketio.NewReadWriter(ws, h.Silent) | ||
|
|
||
| for { | ||
| m := clientMessage{} | ||
| if err := rw.ReadJSON(&m); err != nil { | ||
| log.Println(err) | ||
| break | ||
| } | ||
|
|
||
| switch m.Action { | ||
| case "run": | ||
| cfg, err := parseConfig(m.Data) | ||
| if err != nil { | ||
| log.Println(err) | ||
| break | ||
| } | ||
|
|
||
| go h.service.doRun(rw, cfg) | ||
|
|
||
| case "cancel": | ||
| if ok := h.service.cancelRun(); !ok { | ||
| rw.WriteJSON(errorMessage{Event: "error", Error: "not running"}) //nolint:errcheck | ||
| } | ||
|
|
||
| default: | ||
| rw.WriteTextMessage(fmt.Sprintf("unknown procedure: %s", m.Action)) //nolint:errcheck | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // TODO Update package configparse for this purpose. | ||
|
|
||
| func parseConfig(data configparse.UnmarshaledConfig) (runner.Config, error) { | ||
| p, err := json.Marshal(data) | ||
| if err != nil { | ||
| return runner.Config{}, err | ||
| } | ||
|
|
||
| cfg, err := configparse.JSON(p) | ||
| if err != nil { | ||
| log.Println(err) | ||
| return runner.Config{}, err | ||
| } | ||
|
|
||
| return cfg, nil | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "github.com/benchttp/engine/internal/configparse" | ||
| "github.com/benchttp/engine/runner" | ||
| ) | ||
|
|
||
| type clientMessage struct { | ||
| Action string `json:"action"` | ||
| // Data is non-empty if field Action is "run". | ||
| Data configparse.UnmarshaledConfig `json:"data"` | ||
| } | ||
|
|
||
| type progressMessage struct { | ||
| Event string `json:"state"` | ||
| Data string `json:"data"` // TODO runner.RecordingProgress | ||
| } | ||
|
|
||
| type doneMessage struct { | ||
| Event string `json:"state"` | ||
| Data runner.Report `json:"data"` | ||
| } | ||
|
|
||
| type errorMessage struct { | ||
| Event string `json:"state"` | ||
| Error string `json:"error"` | ||
| } | ||
moreirathomas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be in favor of introducing actual authentication logics when they're specced rather than faking it now 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thing is we need to have a mean to go around the cors.
So the token is needed in this pr.
Are you in favor of not even passing it as a argument from main (where it will be passed from anyway) and in-line it in
server?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can still return
trueinCheckOriginfor now and implement the token logics later (I don't see how it is different from using a token that obtainable from the repo)Regarding its implementation I'd keep the current one, except we get
tokenfrom an environment variable instead of a hardcoded public value.