polygon-io / client-go

The official Go client library for the Polygon REST and WebSocket API.
MIT License
127 stars 36 forks source link

Fix JSON Marshaling for Millis and Nanos Types #368

Closed justinpolygon closed 9 months ago

justinpolygon commented 9 months ago

This PR addresses two bugs https://github.com/polygon-io/client-go/issues/362 & https://github.com/polygon-io/client-go/issues/360 related to the JSON marshaling of Millis and Nanos types. Both users experienced issues where these types were not correctly marshaled into JSON, resulting in empty field in the JSON output.

// Stocks - Aggregates (Bars)
// https://polygon.io/docs/stocks/get_v2_aggs_ticker__stocksticker__range__multiplier___timespan___from___to
// https://github.com/polygon-io/client-go/blob/master/rest/aggs.go
package main

import (
    "context"
    "log"
    "os"
    "time"
    "encoding/json"
    "fmt"

    polygon "github.com/polygon-io/client-go/rest"
    "github.com/polygon-io/client-go/rest/models"
    "github.com/davecgh/go-spew/spew"
)

func main() {

    // init client
    c := polygon.New(os.Getenv("POLYGON_API_KEY"))

    // set params
    params := models.ListAggsParams{
        Ticker:     "AAPL",
        Multiplier: 1,
        Timespan:   "day",
        From:       models.Millis(time.Date(2023, 12, 1, 0, 0, 0, 0, time.UTC)),
        To:         models.Millis(time.Date(2023, 12, 1, 0, 0, 0, 0, time.UTC)),
    }.WithOrder(models.Desc).WithLimit(50000).WithAdjusted(true)

    // make request
    iter := c.ListAggs(context.Background(), params)

    // do something with the result
    for iter.Next() {
        //log.Print(iter.Item())

        bytes, err := json.Marshal(iter.Item())
        fmt.Println(string(bytes))
        fmt.Println(err)

        // dump the struct
        spew.Dump(iter.Item())

        // Convert Nanos to time.Time
        timestamp := time.Time(iter.Item().Timestamp) //res.Results.ParticipantTimestamp)
        //timestamp := iter.Item().Timestamp

        // Print the Unix millisecond timestamp (int64)
        log.Printf("Unix millisecond timestamp: %d", timestamp.UnixMilli())

        // Print the datetime
        log.Printf("Datetime: %v", timestamp)

    }
    if iter.Err() != nil {
        log.Fatal(iter.Err())
    }

}

Here's the incorrectly marshaled JSON response with the missing t value.

{"c":189.95,"h":190.32,"l":188.19,"n":486786,"o":189.84,"t":{},"v":48744366,"vw":189.337}
<nil>
(models.Agg) {
 Ticker: (string) "",
 Close: (float64) 189.95,
 High: (float64) 190.32,
 Low: (float64) 188.19,
 Transactions: (int64) 486786,
 Open: (float64) 189.84,
 Timestamp: (models.Millis) {
  wall: (uint64) 0,
  ext: (int64) 63836917200,
  loc: (*time.Location)(0x1054207e0)(Local)
 },
 Volume: (float64) 4.8744366e+07,
 VWAP: (float64) 189.337,
 OTC: (bool) false
}
2023/12/04 16:53:24 Unix millisecond timestamp: 1701320400000
2023/12/04 16:53:24 Datetime: 2023-11-29 21:00:00 -0800 PST

The core issue was rooted in the use of pointer receivers (*Millis and *Nanos) for the MarshalJSON methods in these types. In our response structs, for Agg, Trades, Quotes, Snapshot, etc, Millis and Nanos were utilized as non-pointer fields (e.g. Timestamp Millis in the response struct). Consequently, when json.Marshal was called, the custom MarshalJSON methods were not being triggered because of the pointer receiver vs. value receiver miss-match, leading to the observed marshaling problems. I was able to debug this by adding fmt.Println statements to MarshalJSON for both Millis and Nanos and discovered these were never being triggered.

To resolve this, the MarshalJSON methods for both Millis and Nanos have been modified to use value receivers instead of pointer receivers. This change ensures that these methods are appropriately invoked during JSON marshaling, even when Millis and Nanos are used as non-pointer fields. This update aligns the method definitions with the actual usage of these types in our response structs and ensures correct JSON serialization behavior.

Here's the correctly marshaled JSON response with t having the correct value after re-running the script above.

{"c":189.95,"h":190.32,"l":188.19,"n":486786,"o":189.84,"t":1701320400000,"v":48744366,"vw":189.337}
<nil>
(models.Agg) {
 Ticker: (string) "",
 Close: (float64) 189.95,
 High: (float64) 190.32,
 Low: (float64) 188.19,
 Transactions: (int64) 486786,
 Open: (float64) 189.84,
 Timestamp: (models.Millis) {
  wall: (uint64) 0,
  ext: (int64) 63836917200,
  loc: (*time.Location)(0x100fd87e0)(Local)
 },
 Volume: (float64) 4.8744366e+07,
 VWAP: (float64) 189.337,
 OTC: (bool) false
}
2023/12/04 16:51:12 Unix millisecond timestamp: 1701320400000
2023/12/04 16:51:12 Datetime: 2023-11-29 21:00:00 -0800 PST

This update specifically targets the JSON marshaling process and should not affect other areas of the application where Millis and Nanos are used. I ran though all the examples for stocks, options, indices, forex, and crypto without issue.

justinpolygon commented 9 months ago

Good catch, and thanks for the thorough explanation 👍

I was surprised and looked into it some more and found a more formalized explanation for this behavior in the Go spec if you're curious.

Great, thanks. Yeah, I was really surprised too.