*> Any help and comments about my application design*
A few general observations first:
1. In several places you are ignoring the error status, in particular at
json.Unmarshal(bodybytes, &nfp) and the error from transport.Client.Do
2. In HandlePUTMessage you are ignoring the HTTP status code
3. I can't see why you're using transport.Client.Do in one function, and
Client.Do in another
4. The "Location" response header, that you read in
SendPUTMessageToServerB, should only be set on 3xx (redirect) responses,
but you only accept 200 / 204
5. Your main function starts a goroutine ("go
SendPATCHMessageEveryTimerValue()"), but then immediately exits:
https://play.golang.org/p/dLXvxfCaki4 . If there's no other stuff going
on, then just remove the "go". (But maybe this is because this is an
incomplete program fragment, and your real main() does other stuff)
6. You have clearly realised that you can't have two different goroutines
reading and writing the same variable concurrently. However the way you've
used mutex seems wrong here. Firstly, you're keeping the mutex locked
indefinitely (you should only lock it for a short time, while performing an
action that touches those variables), and secondly, I don't see any code in
the main goroutine which would be touching the same data concurrently -
which would also need to be protected by the mutex. But again, this could
also be because the program is incomplete.
*> restarting from step 1 when 404 status code is received does not work*
I believe the problem is that you're not returning an "error" status from
PATCHMessage under that condition. You can synthesise one of your own:
} else if status == http.StatusNotFound {
// Here I would like to restart PUT message if status code is 404
*return fmt.Errorf("Got http.StatusNotFound")*
}
... which would then be caught by "if err != nil" in
SendPATCHMessageEveryTimerValue.
If you want, you could make your own object (compatible with the "error"
interface) to carry the HTTP status as a value:
type HttpError struct {
Code int
Description string
}
func (e HttpError) Error() string {
return e.Description
}
...
if status != http.StatusOK {
return &HttpError{
Code: resp.StatusCode,
Description: fmt.Sprintf("Unexpected status: %s",
resp.Status),
}
}
This can be useful if the caller wants to see the HTTP status value and act
on it:
err := Foo()
if e, ok := err.(*HttpError); ok && e.Code == http.StatusForbidden {
.... do something
}
However, personally, I think you should structure the code more closely
along the lines of your original description of the problem.
What I mean is: instead of having SendPATCHMessageEveryTimerValue
internally call out to SendPUTMessage if there's an error, I'd make it
terminate. The caller is then responsible for going back to step 1 and
sending the PUT before sending more PATCHes.
This gives the overall structure something like this:
func PutAndPatch() {
for {
var interval time.Duration
for {
... do your step 1, 2, 3
... break out when you have the valid PUT response and have set
'interval'
}
for {
... do your step 4, 5, 6
... break out when there is a failure to PATCH
}
}
}
The caller can then choose to run this as a go routine, with "go
PutAndPatch()", or not, as they prefer. Note also that the state ("var
interval") is private to this function, so there's no need for a mutex.
This would also allow you to run multiple instances of PutAndPatch, talking
to different servers, in different goroutines.
Of course, as it stands, if you run PutAndPatch() in a goroutine then it
will run autonomously, and the main program will have no idea of its status
- and not even have a way to stop it. If you want to add that sort of
functionality, then the cleanest way is likely to be using channels. For
example, the standard way to signal a "stop" is to have an empty channel,
which the caller closes when they want the stop to take place; or to use
the "context <https://blog.golang.org/context>" library which is a wrapper
around this idea. For any sort of concurrency it's also well worth getting
your head around the contents of this video: GopherCon 2018: Bryan C. Mills
- Rethinking Classical Concurrency Patterns
<https://www.youtube.com/watch?v=5zXAHh5tJqQ>.
If you do decide to read and/or modify global state from PutAndPatch(),
then you'd want to lock the mutex just before accessing it and unlock it
immediately afterwards - and the main program would need to do the same.
It's very easy to introduce bugs if you forget to do this. Use the race
detector <https://blog.golang.org/race-detector> to help find such problems.
HTH,
Brian.
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/golang-nuts/0208f9d2-62df-4bfd-b1d5-b61b5af53e80n%40googlegroups.com.
