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

feat(cfapi): include Ray ID in signing errors #111

Merged
merged 1 commit into from
Jan 21, 2024
Merged

Conversation

terinjokes
Copy link
Contributor

To allow for customer support and engineering teams to correlate failures to API requests from failure reports, include the request's Ray ID in error messages

To allow for customer support and engineering teams to correlate
failures to API requests from failure reports, include the request's Ray
ID in error messages
@terinjokes terinjokes requested a review from a team January 17, 2024 23:12
api := APIResponse{}
if err := json.NewDecoder(resp.Body).Decode(&api); err != nil {
return nil, err
}

if !api.Success {
return nil, &api.Errors[0]
err := &api.Errors[0]
err.RayID = rayID
Copy link
Contributor Author

@terinjokes terinjokes Jan 17, 2024

Choose a reason for hiding this comment

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

The other idea I had was to keep APIError unchanged and return a wrapped error instead. Thoughts?

Choose a reason for hiding this comment

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

It looks like all of the cloudflare-go errors include a RayID field, so I think you're fine to keep doing the same thing here.

api := APIResponse{}
if err := json.NewDecoder(resp.Body).Decode(&api); err != nil {
return nil, err
}

if !api.Success {
return nil, &api.Errors[0]
err := &api.Errors[0]
err.RayID = rayID

Choose a reason for hiding this comment

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

It looks like all of the cloudflare-go errors include a RayID field, so I think you're fine to keep doing the same thing here.

@terinjokes terinjokes merged commit 3954a9e into trunk Jan 21, 2024
13 checks passed
@terinjokes terinjokes deleted the terin/ray-id branch January 21, 2024 17:38
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.

None yet

2 participants