hackgvl / hackgreenville-com

HackGreenville's Website
https://hackgreenville.com
MIT License
16 stars 15 forks source link

Bug: Import Events Yields 'Undefined array key "state"' Exception on Event with no State Set #225

Open allella opened 4 months ago

allella commented 4 months ago

I actually tried to run the importer by hand using php artisan import:events, to test a workaround for https://github.com/hackgvl/hackgreenville-com/issues/213, on production and it processed some orgs and events, up through Greenville SC WordPress Meetup Group and then seems to fail on Greenville Three.js Meetup Group

The first record in the Meetup.com REST API for Greenville Three.js Meetup Group has no "State" value set in the venue. The other records do have a state set, but this is obviously causing an error on the importer.

image

image

allella commented 4 months ago

Actually, I see all of the venue records for Greenville Three.js Meetup Group in the Meetup REST API are missing a state.

I'll connect with Nate to update that as a work around, but it seems we'll need to catch errors for missing values like this on the Laravel side.

allella commented 4 months ago

@bogdankharchenko thanks for #226

It got the importer working again, but I'm curious if Three.js events will continue to have an import issue if that missing state / zip remained.

Nate and I are talking in the Slack about it, and it may be that he's using the mobile app and it doesn't ask for a state, just a zip code.

nate-three-js-meetupbug

allella commented 4 months ago

Also, I didn't see an exception when running the importer live after the new PR.

I guess that's because the error was caught but not directed to the logs?

Processing Page <1> from MeetupRest for Greenville Three.js Meetup Group Importing event: Three.js Meetup @ 2024-02-10 11:00:00 Importing event: Three.js Meetup @ 2024-02-24 11:00:00 Importing event: Three.js Meetup @ 2024-03-09 11:00:00 Importing event: Three.js Meetup @ 2024-03-23 11:00:00 Importing event: Three.js Meetup @ 2024-04-06 11:00:00 Importing event: Three.js Meetup @ 2024-04-20 11:00:00 Importing event: Three.js Meetup @ 2024-05-04 11:00:00 Importing event: Three.js Meetup @ 2024-05-18 11:00:00 Importing event: Three.js Meetup @ 2024-06-01 11:00:00 Importing event: Three.js Meetup @ 2024-06-15 11:00:00 Importing event: Three.js Meetup @ 2024-06-29 11:00:00 Importing event: Three.js Meetup @ 2024-07-13 11:00:00 Importing event: Three.js Meetup @ 2024-07-27 11:00:00 Importing event: Three.js Meetup @ 2024-08-10 11:00:00

allella commented 4 months ago

The fix on the Meetup.com / data side from Nate / Three.js was "I changed it on the desktop site. Before, I searched and selected the location but this time I manually copy pasted the full address"

This corrected the missing State and Zip Code in the Venue in Meetup REST API

image

allella commented 4 months ago

This issue is also happening with at least one Synergy Mill event that just passed on Feb 4, 2024

image

allella commented 4 months ago

@jhloman the Bass Day V event from Feb 4 was missing a state and zip code on the "venue".

Nate, one of the other organizers, was able to fix the issue by updating the venue on the event. Would you mind giving that a try and takes some screenshots of the process?

We are catching and logging this error in the app, but I suspect it will continue to happen from time to time and it would help to document the steps organizers can take to fix it so we can fix the problem in the data, instead of hacking around the bad data.

allella commented 3 months ago

@bogdankharchenko in another wrinkle on this issue, we are seeing duplicates in the events API for Three.js as seen in the calendar and #events channel digest.

Nate did edit the venue to add a zip code and state and perhaps that triggered new records on the import from the venue change?

The Meetup API looks to not have duplicates, so it doesn't seem that causing any direct issue with duplicates https://api.meetup.com/greenville-webgl-meetup-group/events?&sign=true&photo-host=public

allella commented 3 months ago

It sounds like a change to recurring events is the most likely cause, since this didn't seem to happen immediately after the venue change, and since Nate is saying he switched to recurring

image

zach2825 commented 2 months ago

Hey guys,

The relavent lines related to this issue, the fix is good because it'll prevent it from bombimg out. But, It could be handled with an Arr::get accessor or using PHPs ?? '' fallback syntax

So the lines are currently

         return VenueData::from([
                'id' => $data['venue']['id'],
                'name' => $data['venue']['name'],
                'address' => $data['venue']['address_1'] ?? '',
                'city' => $data['venue']['city'],
                'state' => $data['venue']['state'],
                'zip' => $data['venue']['zip'],
                'country' => $data['venue']['country'],
                'lat' => $data['venue']['lat'],
                'lon' => $data['venue']['lon'],
            ]);

they could be

         return VenueData::from([
                'id' => $data['venue']['id'],
                'name' => $data['venue']['name'],
                'address' => $data['venue']['address_1'] ?? '',
                'city' => $data['venue']['city'] ?? '',
                'state' => $data['venue']['state'] ?? '',
                'zip' => $data['venue']['zip'] ?? '',
                'country' => $data['venue']['country'] ?? '',
                'lat' => $data['venue']['lat'] ?? '',
                'lon' => $data['venue']['lon'] ?? '',
            ]);

So, we'd catch all the optional params like state, zip, and country for example and default them to empty string or null to prevent the undefined errors from popping up.

Take note in this same test code image

See after adding ?? '' The test passes now. I also added an equals check, you can see that it defaults to n/a now image

allella commented 2 months ago

Thanks @zach2825 this would certainly help avoid the the bloat in the error logs. If allowing the missing data isn't going to cause any issues downstream, then defaulting to empty strings may do the trick.

The only app I can think using the city, state, zip so far is the Slackbot. So, if we did empty strings we'd probably actually get an easy way to catch future events with missing values. Though, perhaps in the long run we'd want to figure a more proactive way to identify bad upstream events. @oliviasculley is a missing city, state, or zip in the Events API, due to missing data from Meetup or Eventbrite, going to mess up the Slackbot if we're more permissive about allowing events even if these values are not complete?

@bogdankharchenko are there any foreseen reasons why passing empty strings in a venue is going to cause trouble either in the app itself, or in API consuming applications?

I briefly started to mess around with a custom catch to log the offending organization and meetup event. That could be a new feature, instead of expanding this bug fix, but it may be helpful to know when events are giving us missing values so we can give the organizers a heads up to investigate / fix things upstream. So, we could do something like check if any of the values are unexpectedly empty on import and log a minimal set if info in the logs so we can have another way to be proactive with giving organizers a heads up to fix their event info.

allella commented 2 months ago

@bogdankharchenko Zach's changes further improved avoiding this error for undefined venue values and such. Though, I did have a question in the last comment about if setting these undefined values is going to cause any other unexpected issues with the importing.