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

fix: reduce middleware syntax error logs, send to logs, tests for CSP in each env #213

Merged
merged 15 commits into from
Oct 14, 2024

Conversation

stefl
Copy link
Contributor

@stefl stefl commented Oct 10, 2024

Description

  • Reduces the Syntax Error logging we currently have in Sentry, but sends them to Datadog so we still have the data
  • Adds more information on the request
  • Typescript was complaining in the CSP file when I added tests to test middleware behaviour
  • Slightly refactored the CSP file so that we can pass in config rather than using environment variables
  • Added tests to ensure the CSP logic is correct

Copy link

vercel bot commented Oct 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
oak-ai-lesson-assistant ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 14, 2024 10:52am

@stefl stefl changed the title fix: reduce middleware syntax error logs, send to Axiom fix: reduce middleware syntax error logs, send to logs Oct 10, 2024
@@ -149,7 +152,7 @@ export async function authMiddleware(
return response;
}
} catch (error) {
console.error("Error in authMiddleware", error);
console.error({ event: "middleware.auth.error", error });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we log JSON then Datadog is able to query it

Copy link

github-actions bot commented Oct 10, 2024

Playwright e2e tests

Job summary

Download report

To view traces locally, unzip the report and run:

npx playwright show-report ~/Downloads/playwright-report

return NextResponse.json({ error: "Internal Server Error" }, { status: 500 });
}

const cspConfig: CspConfig = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving this to a config allows us to test the CSP generation without relying on ENV being set.

const sentryEnv = process.env.NEXT_PUBLIC_SENTRY_ENV;
const sentryRelease = process.env.NEXT_PUBLIC_APP_VERSION;
const sentryReportUri = `${process.env.SENTRY_REPORT_URI}&sentry_environment=${sentryEnv}&sentry_release=${sentryRelease}`;
function generateNonce(): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not too happy about this. Because it's an edge function, and we're running the test in node we need a way to generate the nonce in both environments.

@stefl stefl changed the title fix: reduce middleware syntax error logs, send to logs fix: reduce middleware syntax error logs, send to logs, tests for CSP in each env Oct 11, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are snapshots so that we can review if there are any changes to how the CSP is generated

@@ -0,0 +1,14 @@
default-src 'self'
media-src 'self' 'self' https://*.mux.com https://stream.mux.com blob:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@codeincontext is this a bug? Should there be as string after blob:?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a bug, just a bit of an oddity to represent a scheme
CleanShot 2024-10-14 at 12 26 29@2x

const savedPolicies = readPoliciesFromFile(env);
expect(generatedPolicies).toBe(savedPolicies);
}
expect(generatedPolicies).toMatchSnapshot();
Copy link
Collaborator

Choose a reason for hiding this comment

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

So nice!

Copy link

@stefl stefl merged commit d18a7d2 into main Oct 14, 2024
12 of 13 checks passed
@stefl stefl deleted the fix/middleware_syntax branch October 14, 2024 10:56
@oak-machine-user
Copy link
Collaborator

🎉 This PR is included in version 1.10.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

3 participants