Skip to content

Conversation

frrist
Copy link
Member

@frrist frrist commented Feb 11, 2022

@frrist frrist requested review from placer14 and iand February 11, 2022 01:25
@frrist frrist force-pushed the frrist/fix-process-error-message branch from 988f3e7 to 458e7ea Compare February 11, 2022 01:26
"go.opencensus.io/tag"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"golang.org/x/xerrors"
Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing changing here, just updated my editor.

Copy link
Contributor

@placer14 placer14 left a comment

Choose a reason for hiding this comment

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

GTM

res.Report[idx].ErrorsDetected = &struct {
Error string
}{Error: e.Error()}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth putting a placeholder for Error if it can't be extracted? Something like "invalid/empty error detail" or similar?

Copy link
Member Author

Choose a reason for hiding this comment

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

Humm we could probably do that, but it would mean enforcing everything that writes to the struct filed being an error since it can contain types that do not implement the error interface:

type ActorStateError struct {
Code string
Name string
Head string
Address string
Error string
}

I'm not sure what the best way to enforce that would be, I'm not sure if the ErrorsDetected filed can be type error due to go-pg. What do you think?

Copy link
Member Author

@frrist frrist Feb 11, 2022

Choose a reason for hiding this comment

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

I'm going to get this merged, but please continue here if anything comes to mind.

@frrist frrist merged commit 67aafb5 into master Feb 11, 2022
@frrist frrist deleted the frrist/fix-process-error-message branch February 11, 2022 23:28
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.

Visor Processing Reports Empty Errors Detected field for message and msapproval tasks
3 participants