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

Logging BedrockError.cause needs to be improved #108

Open
mattcollier opened this issue Nov 4, 2022 · 0 comments
Open

Logging BedrockError.cause needs to be improved #108

mattcollier opened this issue Nov 4, 2022 · 0 comments
Labels
Priority 2 Important but not critical

Comments

@mattcollier
Copy link
Contributor

bedrock/lib/util.js

Lines 80 to 82 in eaac0b9

if(err.cause && !_isErrorPublic(err.cause)) {
object.cause = null;
}

This code assumes that every cause would be a BedrockError that would be explicitly marked as public.

It is not common practice to wrap every error that might be included in a BedrockError in another BedrockError.

I think the practical solution here is to say that the cause should be logged if the outer BedrockError is marked as public.

This of course could mean that a cause leaks data that should not be exposed in logs. However, the current state of affairs makes it impossible to debug applications issues in deployments.

@mattcollier mattcollier changed the title Loggine BedrockError.cause needs to be improved Logging BedrockError.cause needs to be improved Nov 4, 2022
@mandyvenables mandyvenables added the Priority 1 Critical label Nov 9, 2022
@dlongley dlongley added Priority 2 Important but not critical and removed Priority 1 Critical labels Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 2 Important but not critical
Projects
None yet
Development

No branches or pull requests

3 participants