-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
invalid memory address error when joining github_user on a column with null value #233
Comments
golang seems pretty bad when nil pointer exceptions occur - it can be very hard to trace the issue. I need to dig into best what the best practice approach for this would be |
@judell this is an sdk bug, transferring |
@kaidaguerre thanks for the quick action here.
I'm a newbie but yeah, that's my observation so far. It's tempting to gripe about ignoring lots of experience with exception handling in other languages, but I'm trying to keep an open mind here, and will be interested to see how this evolves as we go forward. |
repro
background
I was revisiting the scripts at https://github.com/turbot/steampipe-samples/tree/main/github-external-contributor-analysis, and they were failing for the above reason.
I added a null check, e.g. https://github.com/turbot/steampipe-samples/blob/main/github-external-contributor-analysis/aws_cdk.sql#L48, which solved the problem, but had to do a lot of printf-style debugging in the github plugin in order to track it down.
issue-specific discussion
The scripts haven't changed since I wrote them, and I would guess there were always commits with null
author_login
, so I wondered if this was a regression since v.9 which was the version in play when the scripts first ran successfully. However I tried rolling back to that Steampipe/github-plugin combo and was still hitting the error, so I dunno.general discussion
It seems generally hard to assemble context around this class of error when it happens. I'm wondering: is that just how it goes with golang (terrible pun, sorry), or are there ways the app could deliver more context when this happens.
The text was updated successfully, but these errors were encountered: