PAN-2529-Sample-Gorgias-Shipbob-in-Go - #19
Conversation
13b7054 to
25616b6
Compare
There was a problem hiding this comment.
My only true blocker here is the lack of support for a .env file, particularly given that the readme doesn't match the code.
But I would encourage you to also consider a signal handler context for the timeout watcher, a struct for webhooks, and simplifying the email parser -- I think these will make the code a lot nicer.
I also strongly suggest going through the comments and removing references to python, javascript, and other implementations. Someone reading this integration is probably only going to read this one, the others aren't relevant.
| **The run-limit deadline** is `time.AfterFunc`, not a signal handler. A `context.WithTimeout` | ||
| was considered and rejected: cancelling a context makes the *next* HTTP call return an | ||
| error, which looks like a failure — but the actual desired behavior on timeout is "stop | ||
| cleanly, flush the cursor, and succeed." `time.AfterFunc` schedules a callback on its own | ||
| goroutine without touching anything in flight, which is the right shape for "succeed early" | ||
| rather than "fail early": |
There was a problem hiding this comment.
My robot disagrees with yours here:
Use a context.Context for the run deadline instead of a watchdog goroutine... the
loop can check ctx.Err() between orders and return the cursor normally,
which is exactly what the C# port does with its cancellation token. That one
change removes the mutex around the cursor, the time.AfterFunc
goroutine, the injected Exit and ArmWatchdog seams, and the stdout-capture
plus recover-from-panic scaffolding in the timeout test. It also lets requests be
built with NewRequestWithContext, so an in-flight call is torn down at
the deadline. A Go reader expects context here, and it makes the Pandium
point clearer: a run that stops itself early is still a success.
| // ShipmentID reads the shipment id off a webhook event. ShipBob names it "id" on | ||
| // the webhook body; older docs and some topics call it "shipment_id". Accept | ||
| // either. | ||
| func ShipmentID(event map[string]any) string { |
There was a problem hiding this comment.
Maybe add a Struct for webhook events? Would do away with deepGet and a lot of the work this file is doing with types
|
|
||
| // emailRE mirrors the check the older integration used, so a recipient email | ||
| // found here is one Gorgias would actually accept. | ||
| var emailRE = regexp.MustCompile(`^([-!#-'*+/-9=?A-Z^-~]+(\.[-!#-'*+/-9=?A-Z^-~]+)*|"([\]!#-[^-~ \t]|(\\[\t -~]))+")@([-!#-'*+/-9=?A-Z^-~]+(\.[-!#-'*+/-9=?A-Z^-~]+)*|\[[\t -Z^-~]*])$`) |
There was a problem hiding this comment.
claude pointed me to go's standard library support for email (https://pkg.go.dev/net/mail) apparently this entire function could be replaced with a call to mail.ParseAddress and a check that the parsed address equals the input.
25a59bb to
3ab53c5
Compare
2 participants
PAN-2529