Propagate errors from JSON response #171

knyar · 2018-09-19T10:36:39Z

This fixes #170.

ojongerius

LGTM!

ojongerius · 2018-09-23T23:07:57Z

Hmm integrations test fail with these changes. Not sure why yet:

github.com/zorkian/go-datadog-api  664a1e7 ✔                                                    4d
▶ make testacc
go test integration/* -v  -timeout 90m
=== RUN   TestInvalidAuth
--- PASS: TestInvalidAuth (2.36s)
=== RUN   TestValidAuth
--- PASS: TestValidAuth (0.39s)
=== RUN   TestBaseUrl
=== RUN   TestBaseUrl/Base_url_defaults_to_https://app.datadoghq.com
=== RUN   TestBaseUrl/Base_url_defaults_DATADOG_HOST_environment_variable_if_set
=== RUN   TestBaseUrl/Base_url_can_be_set_through_the_attribute_setter
--- PASS: TestBaseUrl (0.00s)
    --- PASS: TestBaseUrl/Base_url_defaults_to_https://app.datadoghq.com (0.00s)
    --- PASS: TestBaseUrl/Base_url_defaults_DATADOG_HOST_environment_variable_if_set (0.00s)
    --- PASS: TestBaseUrl/Base_url_can_be_set_through_the_attribute_setter (0.00s)
=== RUN   TestDashboardListsGet
--- PASS: TestDashboardListsGet (2.82s)
=== RUN   TestDashboardListCreateAndDelete
--- PASS: TestDashboardListCreateAndDelete (2.15s)
=== RUN   TestDashboardListUpdate
--- PASS: TestDashboardListUpdate (4.09s)
=== RUN   TestDashboardListItemsGetAndUpdate
--- PASS: TestDashboardListItemsGetAndUpdate (7.46s)
=== RUN   TestDashboardListItemsAddAndDelete
--- PASS: TestDashboardListItemsAddAndDelete (8.39s)
=== RUN   TestDashboardCreateAndDelete
--- PASS: TestDashboardCreateAndDelete (4.20s)
=== RUN   TestDashboardCreateAndDeleteAdvancesTimeseries
--- PASS: TestDashboardCreateAndDeleteAdvancesTimeseries (2.46s)
=== RUN   TestDashboardUpdate
--- PASS: TestDashboardUpdate (3.68s)
=== RUN   TestDashboardGet
--- PASS: TestDashboardGet (2.92s)
=== RUN   TestDashboardCreateWithCustomGraph
--- PASS: TestDashboardCreateWithCustomGraph (2.05s)
=== RUN   TestDowntimeCreateAndDelete
--- PASS: TestDowntimeCreateAndDelete (1.77s)
=== RUN   TestDowntimeLinkedToMonitorCreateAndDelete
--- PASS: TestDowntimeLinkedToMonitorCreateAndDelete (4.73s)
=== RUN   TestDowntimeUpdate
--- PASS: TestDowntimeUpdate (3.39s)
=== RUN   TestDowntimeGet
--- FAIL: TestDowntimeGet (0.51s)
	downtime_test.go:75: Retrieving downtimes failed when it shouldn't: json: cannot unmarshal array into Go value of type datadog.Response
=== RUN   TestHost_Mute
--- PASS: TestHost_Mute (1.05s)
=== RUN   TestIntegrationPDCreateAndDelete
--- PASS: TestIntegrationPDCreateAndDelete (2.14s)
=== RUN   TestIntegrationPDUpdate
--- PASS: TestIntegrationPDUpdate (3.54s)
=== RUN   TestIntegrationPDGet
--- PASS: TestIntegrationPDGet (2.94s)
=== RUN   TestIntegrationSlackCreateAndDelete
--- PASS: TestIntegrationSlackCreateAndDelete (3.19s)
=== RUN   TestIntegrationSlackUpdate
--- PASS: TestIntegrationSlackUpdate (3.95s)
=== RUN   TestIntegrationSlackGet
--- PASS: TestIntegrationSlackGet (2.44s)
=== RUN   TestMonitorCreateAndDelete
--- PASS: TestMonitorCreateAndDelete (1.77s)
=== RUN   TestMonitorUpdate
--- PASS: TestMonitorUpdate (3.25s)
=== RUN   TestMonitorUpdateRemovingTags
--- PASS: TestMonitorUpdateRemovingTags (2.28s)
=== RUN   TestMonitorGet
--- FAIL: TestMonitorGet (0.60s)
	monitors_test.go:71: Retrieving monitors failed when it shouldn't: json: cannot unmarshal array into Go value of type datadog.Response
=== RUN   TestMonitorGetWithoutNoDataTimeframe
--- FAIL: TestMonitorGetWithoutNoDataTimeframe (0.56s)
	monitors_test.go:91: Retrieving monitors failed when it shouldn't: json: cannot unmarshal array into Go value of type datadog.Response
=== RUN   TestMonitorMuteUnmute
--- PASS: TestMonitorMuteUnmute (4.00s)
=== RUN   TestWidgetAlertValue
--- PASS: TestWidgetAlertValue (2.74s)
=== RUN   TestWidgetChange
--- PASS: TestWidgetChange (3.16s)
=== RUN   TestWidgetGraph
--- PASS: TestWidgetGraph (2.89s)
=== RUN   TestWidgetEventTimeline
--- PASS: TestWidgetEventTimeline (3.49s)
=== RUN   TestAlertWidgetGraph
--- PASS: TestAlertWidgetGraph (5.54s)
=== RUN   TestWidgetHostMap
--- PASS: TestWidgetHostMap (4.67s)
=== RUN   TestWidgetCheckStatus
--- PASS: TestWidgetCheckStatus (3.24s)
=== RUN   TestWidgetIFrame
--- PASS: TestWidgetIFrame (3.24s)
=== RUN   TestWidgetNote
--- PASS: TestWidgetNote (3.17s)
=== RUN   TestWidgetToplist
--- PASS: TestWidgetToplist (3.94s)
=== RUN   TestWidgetEventSteam
--- PASS: TestWidgetEventSteam (3.21s)
=== RUN   TestWidgetImage
--- PASS: TestWidgetImage (4.55s)
=== RUN   TestWidgetFreeText
--- PASS: TestWidgetFreeText (2.77s)
=== RUN   TestWidgetTimeseries
--- PASS: TestWidgetTimeseries (3.51s)
=== RUN   TestWidgetQueryValue
--- PASS: TestWidgetQueryValue (3.03s)
=== RUN   TestScreenboardCreateAndDelete
--- PASS: TestScreenboardCreateAndDelete (2.88s)
=== RUN   TestScreenboardShareAndRevoke
--- FAIL: TestScreenboardShareAndRevoke (2.94s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x12626e1]

goroutine 148 [running]:
testing.tRunner.func1(0xc42069c000)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:742 +0x29d
panic(0x12c7240, 0x14bf2c0)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/panic.go:502 +0x229
github.com/zorkian/go-datadog-api.(*Client).doJsonRequestUnredacted(0xc420010880, 0x132978f, 0x6, 0xc4201300e0, 0x17, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ojongerius/gocode/src/github.com/zorkian/go-datadog-api/request.go:125 +0x3f1
github.com/zorkian/go-datadog-api.(*Client).doJsonRequest(0xc420010880, 0x132978f, 0x6, 0xc4201300e0, 0x17, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ojongerius/gocode/src/github.com/zorkian/go-datadog-api/request.go:73 +0x91
github.com/zorkian/go-datadog-api.(*Client).RevokeScreenboard(0xc420010880, 0x6e236, 0xc4204fa1c0, 0x0)
	/Users/ojongerius/gocode/src/github.com/zorkian/go-datadog-api/screenboards.go:115 +0xe0
command-line-arguments.TestScreenboardShareAndRevoke(0xc42069c000)
	/Users/ojongerius/gocode/src/github.com/zorkian/go-datadog-api/integration/screenboards_test.go:48 +0x2f4
testing.tRunner(0xc42069c000, 0x133f260)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:824 +0x2e0
FAIL	command-line-arguments	144.080s
make: *** [testacc] Error 1

ojongerius

Integration tests fail with these changes. Not ready to merge.

ojongerius · 2018-09-23T23:19:06Z

@knyar you have more context on this than I do, are you good to investigate and fix this by yourself?

knyar · 2018-09-24T09:37:58Z

Apologies for not checking the integration tests. I can't reproduce these errors after rebasing my change, so I suspect they were caused by the fact that I had my patch applied on top of a pretty old commit.

After making some minor changes to screen_widgets_test to make it build and pass (included in this PR as a separate commit) I am not seeing any integration test failures. Could you please check whether things look good on your end?

ojongerius · 2018-09-25T01:06:47Z

Hey @knyar. Can you verify you are able to successfully run make testacc (vs make test) against acc13a2 ? If so, could you please share the output?

You'll need to set DATADOG_API_KEY and DATADOG_APP_KEY for them to run.

Running locally on my laptop, using go1.10.3 darwin/amd64:

github.com/zorkian/go-datadog-api  acc13a2 ✔                                                                    15h28m
▶ make testacc
go test integration/* -v  -timeout 90m
=== RUN   TestInvalidAuth
--- PASS: TestInvalidAuth (1.20s)
=== RUN   TestValidAuth
--- PASS: TestValidAuth (0.42s)
=== RUN   TestBaseUrl
=== RUN   TestBaseUrl/Base_url_defaults_to_https://app.datadoghq.com
=== RUN   TestBaseUrl/Base_url_defaults_DATADOG_HOST_environment_variable_if_set
=== RUN   TestBaseUrl/Base_url_can_be_set_through_the_attribute_setter
--- PASS: TestBaseUrl (0.00s)
    --- PASS: TestBaseUrl/Base_url_defaults_to_https://app.datadoghq.com (0.00s)
    --- PASS: TestBaseUrl/Base_url_defaults_DATADOG_HOST_environment_variable_if_set (0.00s)
    --- PASS: TestBaseUrl/Base_url_can_be_set_through_the_attribute_setter (0.00s)
=== RUN   TestDashboardListsGet
--- PASS: TestDashboardListsGet (1.77s)
=== RUN   TestDashboardListCreateAndDelete
--- PASS: TestDashboardListCreateAndDelete (1.36s)
=== RUN   TestDashboardListUpdate
--- PASS: TestDashboardListUpdate (1.99s)
=== RUN   TestDashboardListItemsGetAndUpdate
--- PASS: TestDashboardListItemsGetAndUpdate (6.68s)
=== RUN   TestDashboardListItemsAddAndDelete
--- PASS: TestDashboardListItemsAddAndDelete (5.95s)
=== RUN   TestDashboardCreateAndDelete
--- PASS: TestDashboardCreateAndDelete (1.94s)
=== RUN   TestDashboardCreateAndDeleteAdvancesTimeseries
--- PASS: TestDashboardCreateAndDeleteAdvancesTimeseries (6.66s)
=== RUN   TestDashboardUpdate
--- PASS: TestDashboardUpdate (2.06s)
=== RUN   TestDashboardGet
--- PASS: TestDashboardGet (1.93s)
=== RUN   TestDashboardCreateWithCustomGraph
--- PASS: TestDashboardCreateWithCustomGraph (2.09s)
=== RUN   TestDowntimeCreateAndDelete
--- PASS: TestDowntimeCreateAndDelete (1.18s)
=== RUN   TestDowntimeLinkedToMonitorCreateAndDelete
--- PASS: TestDowntimeLinkedToMonitorCreateAndDelete (2.21s)
=== RUN   TestDowntimeUpdate
--- PASS: TestDowntimeUpdate (1.43s)
=== RUN   TestDowntimeGet
--- FAIL: TestDowntimeGet (0.28s)
	downtime_test.go:75: Retrieving downtimes failed when it shouldn't: json: cannot unmarshal array into Go value of type datadog.Response
=== RUN   TestHost_Mute
--- PASS: TestHost_Mute (0.57s)
=== RUN   TestIntegrationPDCreateAndDelete
--- PASS: TestIntegrationPDCreateAndDelete (1.50s)
=== RUN   TestIntegrationPDUpdate
--- PASS: TestIntegrationPDUpdate (2.24s)
=== RUN   TestIntegrationPDGet
--- PASS: TestIntegrationPDGet (1.48s)
=== RUN   TestIntegrationSlackCreateAndDelete
--- PASS: TestIntegrationSlackCreateAndDelete (1.37s)
=== RUN   TestIntegrationSlackUpdate
--- PASS: TestIntegrationSlackUpdate (2.28s)
=== RUN   TestIntegrationSlackGet
--- PASS: TestIntegrationSlackGet (1.48s)
=== RUN   TestIntegrationAWSCreateAndDelete
--- PASS: TestIntegrationAWSCreateAndDelete (1.32s)
=== RUN   TestMonitorCreateAndDelete
--- PASS: TestMonitorCreateAndDelete (1.20s)
=== RUN   TestMonitorUpdate
--- PASS: TestMonitorUpdate (1.66s)
=== RUN   TestMonitorUpdateRemovingTags
--- PASS: TestMonitorUpdateRemovingTags (1.49s)
=== RUN   TestMonitorGet
--- FAIL: TestMonitorGet (0.45s)
	monitors_test.go:71: Retrieving monitors failed when it shouldn't: json: cannot unmarshal array into Go value of type datadog.Response
=== RUN   TestMonitorGetWithoutNoDataTimeframe
--- FAIL: TestMonitorGetWithoutNoDataTimeframe (0.35s)
	monitors_test.go:91: Retrieving monitors failed when it shouldn't: json: cannot unmarshal array into Go value of type datadog.Response
=== RUN   TestMonitorMuteUnmute
--- PASS: TestMonitorMuteUnmute (2.04s)
=== RUN   TestWidgets
=== RUN   TestWidgets/free_text
=== RUN   TestWidgets/timeseries
=== RUN   TestWidgets/query_value
=== RUN   TestWidgets/toplist
=== RUN   TestWidgets/change
=== RUN   TestWidgets/event_timeline
=== RUN   TestWidgets/event_stream
=== RUN   TestWidgets/image
=== RUN   TestWidgets/note
=== RUN   TestWidgets/alert_graph
=== RUN   TestWidgets/alert_value
=== RUN   TestWidgets/iframe
=== RUN   TestWidgets/check_status
=== RUN   TestWidgets/trace_service
=== RUN   TestWidgets/hostmap
=== RUN   TestWidgets/manage_status
=== RUN   TestWidgets/log_stream
=== RUN   TestWidgets/uptime
=== RUN   TestWidgets/process
--- PASS: TestWidgets (41.97s)
    --- PASS: TestWidgets/free_text (2.08s)
    --- PASS: TestWidgets/timeseries (1.94s)
    --- PASS: TestWidgets/query_value (2.08s)
    --- PASS: TestWidgets/toplist (2.09s)
    --- PASS: TestWidgets/change (1.95s)
    --- PASS: TestWidgets/event_timeline (2.37s)
    --- PASS: TestWidgets/event_stream (2.28s)
    --- PASS: TestWidgets/image (2.44s)
    --- PASS: TestWidgets/note (1.87s)
    --- PASS: TestWidgets/alert_graph (1.93s)
    --- PASS: TestWidgets/alert_value (2.34s)
    --- PASS: TestWidgets/iframe (2.09s)
    --- PASS: TestWidgets/check_status (2.44s)
    --- PASS: TestWidgets/trace_service (1.97s)
    --- PASS: TestWidgets/hostmap (1.84s)
    --- PASS: TestWidgets/manage_status (3.13s)
    --- PASS: TestWidgets/log_stream (2.76s)
    --- PASS: TestWidgets/uptime (2.26s)
    --- PASS: TestWidgets/process (2.10s)
=== RUN   TestScreenboardCreateAndDelete
--- PASS: TestScreenboardCreateAndDelete (1.64s)
=== RUN   TestScreenboardShareAndRevoke
--- FAIL: TestScreenboardShareAndRevoke (2.04s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1263cd1]

goroutine 460 [running]:
testing.tRunner.func1(0xc4201380f0)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:742 +0x29d
panic(0x12caf00, 0x14c52c0)
	/usr/local/Cellar/go/1.10.3/libexec/src/runtime/panic.go:502 +0x229
github.com/zorkian/go-datadog-api.(*Client).doJsonRequestUnredacted(0xc4200a0800, 0x132abe9, 0x6, 0xc4202204a0, 0x17, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ojongerius/gocode/src/github.com/zorkian/go-datadog-api/request.go:125 +0x3f1
github.com/zorkian/go-datadog-api.(*Client).doJsonRequest(0xc4200a0800, 0x132abe9, 0x6, 0xc4202204a0, 0x17, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ojongerius/gocode/src/github.com/zorkian/go-datadog-api/request.go:73 +0x91
github.com/zorkian/go-datadog-api.(*Client).RevokeScreenboard(0xc4200a0800, 0x6e683, 0xc42017c2c0, 0x0)
	/Users/ojongerius/gocode/src/github.com/zorkian/go-datadog-api/screenboards.go:92 +0xe0
command-line-arguments.TestScreenboardShareAndRevoke(0xc4201380f0)
	/Users/ojongerius/gocode/src/github.com/zorkian/go-datadog-api/integration/screenboards_test.go:48 +0x2f4
testing.tRunner(0xc4201380f0, 0x1340b08)
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
	/usr/local/Cellar/go/1.10.3/libexec/src/testing/testing.go:824 +0x2e0
FAIL	command-line-arguments	104.278s
make: *** [testacc] Error 1

knyar · 2018-09-25T07:28:48Z

Yep, the tests pass when I run them against a fresh Datadog account: https://knyar.net/paste/raw/6c42eb96.txt

I suspect the failures might be caused by specific data present in the account you are using for the tests.

I will try to add more data to my account manually and see if I can make tests fail in a similar way, but if you could grab JSON output from Datadog API (just for TestDowntimeGet, TestMonitorGet, TestMonitorGetWithoutNoDataTimeframe, and TestScreenboardShareAndRevoke) that would help a lot.

ojongerius · 2018-09-25T08:00:30Z

Interesting! @knyar I'm a little busy at the moment, but I've shot you an invite to the DD account we use and you can have a gander yourself. Ping me here if you did not receive it.

This fixes zorkian#170.

knyar · 2018-09-25T13:00:33Z

Thanks! The failures were caused by some API responses being arrays that could not be unmarshalled into a struct. I've adjusted the new error checking code, and tests are passing now.

ojongerius · 2018-09-25T21:20:03Z

Fantastic, that does sound like the root cause. Thanks for the quick turnaround!

ojongerius

LGTM 👍

ojongerius · 2018-09-25T21:34:33Z

Thanks again @knyar, your changes are included in https://github.com/zorkian/go-datadog-api/releases/tag/v2.14.0

knyar force-pushed the errors branch from 40a9769 to 664a1e7 Compare September 19, 2018 10:37

knyar changed the title ~~Propagate errors from JSON response (fix #170)~~ Propagate errors from JSON response. Sep 19, 2018

knyar changed the title ~~Propagate errors from JSON response.~~ Propagate errors from JSON response Sep 19, 2018

ojongerius approved these changes Sep 23, 2018

View reviewed changes

ojongerius requested changes Sep 23, 2018

View reviewed changes

knyar force-pushed the errors branch from 664a1e7 to acc13a2 Compare September 24, 2018 09:37

knyar and others added 2 commits September 25, 2018 13:45

Use json.Number in screen_widgets_test

08b3b61

Propagate errors from JSON response

5282e9f

This fixes zorkian#170.

knyar force-pushed the errors branch from acc13a2 to 5282e9f Compare September 25, 2018 12:57

ojongerius approved these changes Sep 25, 2018

View reviewed changes

ojongerius merged commit 9d88cb5 into zorkian:master Sep 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate errors from JSON response #171

Propagate errors from JSON response #171

knyar commented Sep 19, 2018 •

edited

Loading

ojongerius left a comment

ojongerius commented Sep 23, 2018

ojongerius left a comment

ojongerius commented Sep 23, 2018 •

edited

Loading

knyar commented Sep 24, 2018

ojongerius commented Sep 25, 2018

knyar commented Sep 25, 2018

ojongerius commented Sep 25, 2018

knyar commented Sep 25, 2018

ojongerius commented Sep 25, 2018

ojongerius left a comment

ojongerius commented Sep 25, 2018

Propagate errors from JSON response #171

Propagate errors from JSON response #171

Conversation

knyar commented Sep 19, 2018 • edited Loading

ojongerius left a comment

Choose a reason for hiding this comment

ojongerius commented Sep 23, 2018

ojongerius left a comment

Choose a reason for hiding this comment

ojongerius commented Sep 23, 2018 • edited Loading

knyar commented Sep 24, 2018

ojongerius commented Sep 25, 2018

knyar commented Sep 25, 2018

ojongerius commented Sep 25, 2018

knyar commented Sep 25, 2018

ojongerius commented Sep 25, 2018

ojongerius left a comment

Choose a reason for hiding this comment

ojongerius commented Sep 25, 2018

knyar commented Sep 19, 2018 •

edited

Loading

ojongerius commented Sep 23, 2018 •

edited

Loading