Skip to content

Commit 59b6a6a

Browse files
richardthe3rdclaude
richardthe3rd
and
claude
authored
fix(festivals): skip malformed festival entries instead of crashing (#330)
* fix(festivals): skip malformed festival entries instead of crashing A single bad entry in the festivals.json response caused a hard cast failure that propagated through the entire .map() chain, leaving _festivals = [] and the app completely unusable. Wrap each Festival.fromJson call in a try/catch inside FestivalsResponse.fromJson so that one bad entry is silently skipped while all valid entries load normally. Also guard against a null or missing "festivals" key in the response. Fixes #273 https://claude.ai/code/session_01PvGuP9cwKFcRMaitwrpkJy * fix(festivals): handle non-List type for festivals key defensively The previous cast `(json['festivals'] as List<dynamic>?)` only handled null; a server returning a Map or String for that key would still throw a TypeError. Replace the nullable cast with an `is List` type check so any unexpected type falls back to an empty list without crashing. Addresses review comment on #330. https://claude.ai/code/session_01PvGuP9cwKFcRMaitwrpkJy * style: apply dart format https://claude.ai/code/session_01PvGuP9cwKFcRMaitwrpkJy --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4739cc3 commit 59b6a6a

2 files changed

Lines changed: 169 additions & 11 deletions

File tree

‎lib/services/festival_service.dart‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,27 @@ class FestivalsResponse {
2020

2121
factory FestivalsResponse.fromJson(
2222
Map<String, dynamic> json, String baseUrl) {
23-
final festivalsList = (json['festivals'] as List<dynamic>).map((f) {
24-
final festivalJson = Map<String, dynamic>.from(f as Map<String, dynamic>);
25-
// Resolve relative URLs to absolute URLs
26-
if (festivalJson['data_base_url'] != null) {
27-
final dataBaseUrl = festivalJson['data_base_url'] as String;
28-
if (dataBaseUrl.startsWith('/')) {
29-
festivalJson['data_base_url'] = baseUrl + dataBaseUrl;
30-
}
31-
}
32-
return Festival.fromJson(festivalJson);
33-
}).toList();
23+
final rawFestivals = json['festivals'];
24+
final festivalsList =
25+
(rawFestivals is List ? rawFestivals : const <dynamic>[])
26+
.map<Festival?>((f) {
27+
try {
28+
final festivalJson =
29+
Map<String, dynamic>.from(f as Map<String, dynamic>);
30+
// Resolve relative URLs to absolute URLs
31+
if (festivalJson['data_base_url'] != null) {
32+
final dataBaseUrl = festivalJson['data_base_url'] as String;
33+
if (dataBaseUrl.startsWith('/')) {
34+
festivalJson['data_base_url'] = baseUrl + dataBaseUrl;
35+
}
36+
}
37+
return Festival.fromJson(festivalJson);
38+
} catch (_) {
39+
return null;
40+
}
41+
})
42+
.whereType<Festival>()
43+
.toList();
3444

3545
return FestivalsResponse(
3646
festivals: festivalsList,

‎test/services/festival_service_test.dart‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import 'package:cambridge_beer_festival/services/festival_service.dart';
2+
import 'package:flutter_test/flutter_test.dart';
3+
4+
void main() {
5+
group('FestivalsResponse.fromJson', () {
6+
const baseUrl = 'https://example.com';
7+
8+
Map<String, dynamic> validFestivalJson({String id = 'cbf2025'}) => {
9+
'id': id,
10+
'name': 'Cambridge Beer Festival 2025',
11+
'data_base_url': 'https://data.example.com/$id',
12+
};
13+
14+
test('parses a valid response with one festival', () {
15+
final response = FestivalsResponse.fromJson(
16+
{
17+
'festivals': [validFestivalJson()],
18+
'default_festival_id': 'cbf2025',
19+
},
20+
baseUrl,
21+
);
22+
23+
expect(response.festivals.length, 1);
24+
expect(response.festivals.single.id, 'cbf2025');
25+
expect(response.defaultFestivalId, 'cbf2025');
26+
});
27+
28+
test('skips a festival entry with a null id and returns the valid ones',
29+
() {
30+
final response = FestivalsResponse.fromJson(
31+
{
32+
'festivals': [
33+
validFestivalJson(id: 'cbf2025'),
34+
{
35+
'id': null,
36+
'name': 'Bad Festival',
37+
'data_base_url': 'https://data.example.com/bad'
38+
},
39+
validFestivalJson(id: 'cbf2024'),
40+
],
41+
'default_festival_id': 'cbf2025',
42+
},
43+
baseUrl,
44+
);
45+
46+
expect(response.festivals.length, 2);
47+
expect(response.festivals.map((f) => f.id),
48+
containsAll(['cbf2025', 'cbf2024']));
49+
});
50+
51+
test(
52+
'skips a festival entry with a missing data_base_url and returns the valid ones',
53+
() {
54+
final response = FestivalsResponse.fromJson(
55+
{
56+
'festivals': [
57+
validFestivalJson(id: 'cbf2025'),
58+
{'id': 'bad', 'name': 'No URL Festival'},
59+
validFestivalJson(id: 'cbf2024'),
60+
],
61+
'default_festival_id': 'cbf2025',
62+
},
63+
baseUrl,
64+
);
65+
66+
expect(response.festivals.length, 2);
67+
expect(response.festivals.map((f) => f.id),
68+
containsAll(['cbf2025', 'cbf2024']));
69+
});
70+
71+
test(
72+
'skips a festival entry whose value is not a Map and returns the valid ones',
73+
() {
74+
final response = FestivalsResponse.fromJson(
75+
{
76+
'festivals': [
77+
validFestivalJson(id: 'cbf2025'),
78+
'this is not a map',
79+
42,
80+
validFestivalJson(id: 'cbf2024'),
81+
],
82+
'default_festival_id': 'cbf2025',
83+
},
84+
baseUrl,
85+
);
86+
87+
expect(response.festivals.length, 2);
88+
expect(response.festivals.map((f) => f.id),
89+
containsAll(['cbf2025', 'cbf2024']));
90+
});
91+
92+
test('returns an empty festival list when all entries are malformed', () {
93+
final response = FestivalsResponse.fromJson(
94+
{
95+
'festivals': [
96+
{
97+
'id': null,
98+
'name': 'Bad1',
99+
'data_base_url': 'https://data.example.com/bad'
100+
},
101+
{'name': 'Bad2'},
102+
],
103+
'default_festival_id': 'cbf2025',
104+
},
105+
baseUrl,
106+
);
107+
108+
expect(response.festivals, isEmpty);
109+
});
110+
111+
test('handles a null or missing "festivals" key without throwing', () {
112+
final responseWithNull = FestivalsResponse.fromJson(
113+
{
114+
'festivals': null,
115+
'default_festival_id': 'cbf2025',
116+
},
117+
baseUrl,
118+
);
119+
expect(responseWithNull.festivals, isEmpty);
120+
121+
final responseWithMissing = FestivalsResponse.fromJson(
122+
{'default_festival_id': 'cbf2025'},
123+
baseUrl,
124+
);
125+
expect(responseWithMissing.festivals, isEmpty);
126+
});
127+
128+
test('handles a non-List value for "festivals" key without throwing', () {
129+
final responseWithMap = FestivalsResponse.fromJson(
130+
{
131+
'festivals': {'unexpected': 'map'},
132+
'default_festival_id': 'cbf2025',
133+
},
134+
baseUrl,
135+
);
136+
expect(responseWithMap.festivals, isEmpty);
137+
138+
final responseWithString = FestivalsResponse.fromJson(
139+
{
140+
'festivals': 'not a list',
141+
'default_festival_id': 'cbf2025',
142+
},
143+
baseUrl,
144+
);
145+
expect(responseWithString.festivals, isEmpty);
146+
});
147+
});
148+
}

0 commit comments

Comments
 (0)