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

Use console.error for Prisma errors #8722

Merged
merged 1 commit into from
Aug 13, 2023
Merged

Use console.error for Prisma errors #8722

merged 1 commit into from
Aug 13, 2023

Conversation

dcousens
Copy link
Member

@dcousens dcousens commented Jul 31, 2023

This pull request changes KS_PRISMA_ERRORS to now be logged using console.error on the server.
This isn't a breaking change in any sense, but it might be a change of scope for what users expect from these errors if they output sensitive information.

I am open to feedback on if this is acceptable or not as a minor.

@changeset-bot

This comment was marked as resolved.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 31, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e8287e4:

Sandbox Source
@keystone-6/sandbox Configuration

@molomby
Copy link
Member

molomby commented Aug 1, 2023

Hmm.. I don't have strong opinions on whether this should be breaking or not, I'm more concerned with direct use of console. Can the prismaError function be replaced by devs somehow? If my Prisma errors do contain sensitive info, how do I prevent it being logged after this change? Do I need to redirect and filter the output of the entire process?

Most apps I've deployed prefer to always log JSON blobs (as Keystone 5 did by default with pino). When Keystone refs console directly, such has here, in the start and migrate scripts, etc. it makes this hard. I know it's a larger point but I'd love KS to accept a logger object, supporting the standard log(), info(), error(), etc. interface. It could default to console, just gives the dev control.

Basically, I say we should address #7668 before adding more output of any kind.

@dcousens
Copy link
Member Author

dcousens commented Aug 1, 2023

This is my thoughts too, and @borisno2 and I are working on that in the background, so I'm happy to park this for now.

@dcousens dcousens merged commit 98fbbeb into main Aug 13, 2023
58 checks passed
@dcousens dcousens deleted the prisma-error-stderr branch August 13, 2023 23:49
@dcousens
Copy link
Member Author

Merging, but we need to address #7668 before release

dcousens added a commit that referenced this pull request Aug 15, 2023
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
dcousens added a commit that referenced this pull request Aug 30, 2023
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
dcousens added a commit that referenced this pull request Sep 10, 2023
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
dcousens added a commit that referenced this pull request Mar 28, 2024
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
dcousens added a commit that referenced this pull request Mar 28, 2024
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
dcousens added a commit that referenced this pull request Apr 4, 2024
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
dcousens added a commit that referenced this pull request Apr 8, 2024
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
dcousens added a commit that referenced this pull request Apr 8, 2024
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
dcousens added a commit that referenced this pull request Apr 8, 2024
Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants