Skip to content

fix jira error parsing#39204

Merged
hugoShaka merged 11 commits intomasterfrom
hugo/log-jira-error
Mar 14, 2024
Merged

fix jira error parsing#39204
hugoShaka merged 11 commits intomasterfrom
hugo/log-jira-error

Conversation

@hugoShaka
Copy link
Copy Markdown
Contributor

Fixes #38231

Changelog: Make the Jira access plugin log Jira errors properly.

Comment thread integrations/access/jira/types.go Outdated
Comment thread integrations/access/jira/types_test.go Outdated
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Comment thread integrations/access/jira/types.go Outdated
func (e ErrorResult) Error() string {
sb := strings.Builder{}
if len(e.ErrorMessages) > 0 {
sb.WriteString(fmt.Sprintf("error messages: %s ", e.ErrorMessages))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: Should we maybe string.Join(e.ErrorMessages, ";") or something like this here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I always find those hard to read when multiple error happen, I'd prefer JSON-style array for those, which is done natively by Sprintf %s.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure about the error messages format (do they include some punctuation?), but adding a join char should help with readability.
Otherwise we might end up with a long string without knowing where a particular ErrorMessage starts and where it ends
Eg

V=[issue invalid format error]
using join=issue invalid; format error

Comment thread integrations/access/jira/types.go Outdated
Comment thread integrations/access/jira/types.go Outdated
Comment thread integrations/access/jira/types.go Outdated
hugoShaka and others added 2 commits March 12, 2024 09:48
Co-authored-by: Roman Tkachenko <roman@goteleport.com>
@hugoShaka hugoShaka enabled auto-merge March 12, 2024 22:06
Comment thread integrations/access/jira/types.go Outdated
return trace.Errorf("http error code=%v, errors=[%s]", resp.StatusCode(), result)
case nil:
return nil
default:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the error is >399 (resp.IsError()==true), does it make sense to return nil if no error is defined?

Copy link
Copy Markdown
Contributor Author

@hugoShaka hugoShaka Mar 13, 2024

Choose a reason for hiding this comment

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

I think this should be unreachable, but I'm quite afraid of breaking something 😓 . It feels like this was added on purpose. Maybe for the "request cancelled" kind of issues.

Comment thread integrations/access/jira/types.go Outdated
func (e ErrorResult) Error() string {
sb := strings.Builder{}
if len(e.ErrorMessages) > 0 {
sb.WriteString(fmt.Sprintf("error messages: %s ", e.ErrorMessages))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure about the error messages format (do they include some punctuation?), but adding a join char should help with readability.
Otherwise we might end up with a long string without knowing where a particular ErrorMessage starts and where it ends
Eg

V=[issue invalid format error]
using join=issue invalid; format error

@hugoShaka hugoShaka added this pull request to the merge queue Mar 14, 2024
Merged via the queue into master with commit 0942b6f Mar 14, 2024
@hugoShaka hugoShaka deleted the hugo/log-jira-error branch March 14, 2024 15:15
@public-teleport-github-review-bot
Copy link
Copy Markdown

@hugoShaka See the table below for backport results.

Branch Result
branch/v13 Create PR
branch/v14 Create PR
branch/v15 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Jira plugin fails to unmarshal errors from Jira API

5 participants