Skip to content

Commit e914859

Browse files
view: improve type assertions and handle stream write errors (#33)
* view: type assert the timestamp * view: type assert the renderers Apparently, the forcetypeassert linter wasn’t satisfied with relying on the type assertion from the assert package and required an explicit type assertion instead. * view: panic when the underlying stream can't write Previously, we were suppressing errors when writing to the underlying stream. While there’s no perfect way to handle this, panicking ensures that failures don’t go unnoticed.
1 parent 1ea8ac4 commit e914859

File tree

3 files changed

+21
-7
lines changed

3 files changed

+21
-7
lines changed

internal/view/json_test.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,22 @@ func testJSONViewOutputEqualsFull(t *testing.T, output string, want []map[string
8282
t.Fatal(err)
8383
}
8484

85+
// While we don't control this directly, we can at least ensure that the timestamp
86+
// is present and correctly formatted, as expected.
87+
// The timestamp is added by the logger, not the JSONView.
8588
if timestamp, ok := gotStruct["@timestamp"]; !ok {
8689
t.Errorf("message has no timestamp: %#v", gotStruct)
8790
} else {
8891
// Remove the timestamp value from the struct to allow comparison
8992
delete(gotStruct, "@timestamp")
9093

9194
// Verify the timestamp format
92-
if _, err := time.Parse(time.RFC3339, timestamp.(string)); err != nil {
93-
t.Errorf("error parsing timestamp on line %d: %s", i, err)
95+
if str, ok := timestamp.(string); ok {
96+
if _, err := time.Parse(time.RFC3339, str); err != nil {
97+
t.Errorf("error parsing timestamp on line %d: %s", i, err)
98+
}
99+
} else {
100+
t.Errorf("unexpected type for timestamp on line %d: %T", i, timestamp)
94101
}
95102
}
96103

internal/view/render.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ func NewHumanRenderer(view *View) *HumanRenderer {
3939
// Render renders a DNS record in human-readable format to the output stream.
4040
func (v *HumanRenderer) Render(domain string, record dns.RR) {
4141
humanReadable := formatRecord(domain, record)
42-
v.view.Stream.Writer.Write([]byte(humanReadable + "\n"))
42+
_, err := v.view.Stream.Writer.Write([]byte(humanReadable + "\n"))
43+
if err != nil {
44+
panic(err)
45+
}
4346
}
4447

4548
// JSONRenderer for rendering JSON output.

internal/view/render_test.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@ func TestNewRenderer_human(t *testing.T) {
1818
hv := NewRenderer(arguments.ViewHuman, NewView(&b))
1919

2020
// Check that the view is a HumanRenderer
21-
assert.IsType(t, &HumanRenderer{}, hv)
21+
humanRenderer, ok := hv.(*HumanRenderer)
22+
assert.True(t, ok, "Expected hv to be of type *HumanRenderer")
23+
24+
assert.IsType(t, &HumanRenderer{}, humanRenderer)
2225

2326
// Check that the view's stream writer is the same as the buffer
24-
assert.Equal(t, &b, hv.(*HumanRenderer).view.Stream.Writer)
27+
assert.Equal(t, &b, humanRenderer.view.Stream.Writer)
2528
}
2629

2730
// TestNewHumanRenderer should simply return a HumanRenderer.
@@ -110,10 +113,11 @@ func TestNewRenderer_JSON(t *testing.T) {
110113
jv := NewRenderer(arguments.ViewJSON, NewView(&b))
111114

112115
// Check that the view is a JSONRenderer
113-
assert.IsType(t, &JSONRenderer{}, jv)
116+
jsonRenderer, ok := jv.(*JSONRenderer)
117+
assert.True(t, ok, "Expected jv to be of type *JSONRenderer")
114118

115119
// Check that the view's stream writer is the same as the buffer
116-
assert.Equal(t, &b, jv.(*JSONRenderer).view.Stream.Writer)
120+
assert.Equal(t, &b, jsonRenderer.view.Stream.Writer)
117121
}
118122

119123
// TestNewJSONRenderer should simply return a JSONRenderer.

0 commit comments

Comments
 (0)