Skip to content
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

Catch nil-deref when creating catalog entries #157

Merged
merged 2 commits into from
Jan 30, 2025
Merged

Conversation

louisheath
Copy link
Contributor

@louisheath louisheath commented Jan 30, 2025

A customer reported an error dereferencing result.JSON201.CatalogEntry

All responses to this customer that morning were 201s, and ParseCatalogV2CreateEntryResponse appears solid

It's near impossible to debug this without identifying the exact request, so we're adding nil-handling and will look again if it reappears

@@ -43,6 +43,13 @@ func EntriesClientFromClient(cl *client.ClientWithResponses) EntriesClient {
return nil, err
}

if result == nil {
return nil, errors.New("unexpected nil response")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any metadata we can add here to help debug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the caller will log out the error alongside the external_id

I think from there on we'll need to ask the customer for the rest of the payload they've specified for that external ID

@louisheath louisheath merged commit c73394b into master Jan 30, 2025
1 check passed
@louisheath louisheath deleted the louis/catch-nil branch January 30, 2025 16:40
louisheath added a commit that referenced this pull request Jan 31, 2025
Related to #157

The nil dereference has been avoided however the catalog importer is
still aborted mid-flow.
Despite erroring, the catalog entry is successfully created, so the next
retry succeeds.

I can't reproduce this error locally, and from inspection our app code
does not seem capable of returning an empty payload with a 201 status
code.
The customer reported seeing a 201 status code in their proxy, and our
logs indicate that we responded with a 592 Byte response, which is
larger than many other successful requests from their previous run.

My only remaining theory is that the "Content-Type" header is missing -
either it was never set or it was stripped mid-way.
This would prevent us from marshalling the response in `client.gen.go`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants