Skip to content

fix(node): Make fastify types more broad #11544

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

Merged
merged 3 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
"version": "1.0.0",
"private": true,
"scripts": {
"start": "node src/app.js",
"start": "ts-node src/app.ts",
"test": "playwright test",
"clean": "npx rimraf node_modules pnpm-lock.yaml",
"test:build": "pnpm install",
"typecheck": "tsc",
"test:build": "pnpm install && pnpm run typecheck",
"test:assert": "pnpm test"
},
"dependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,37 +1,50 @@
require('./tracing');
import type * as S from '@sentry/node';
const Sentry = require('@sentry/node') as typeof S;

Sentry.init({
environment: 'qa', // dynamic sampling bias to keep transactions
dsn: process.env.E2E_TEST_DSN,
integrations: [],
tracesSampleRate: 1,
tunnel: 'http://localhost:3031/', // proxy server
tracePropagationTargets: ['http://localhost:3030', '/external-allowed'],
});

import type * as H from 'http';
import type * as F from 'fastify';

const Sentry = require('@sentry/node');
const { fastify } = require('fastify');
const http = require('http');
// Make sure fastify is imported after Sentry is initialized
const { fastify } = require('fastify') as typeof F;
const http = require('http') as typeof H;

const app = fastify();
const port = 3030;
const port2 = 3040;

Sentry.setupFastifyErrorHandler(app);

app.get('/test-success', function (req, res) {
app.get('/test-success', function (_req, res) {
res.send({ version: 'v1' });
});

app.get('/test-param/:param', function (req, res) {
app.get<{ Params: { param: string } }>('/test-param/:param', function (req, res) {
res.send({ paramWas: req.params.param });
});

app.get('/test-inbound-headers/:id', function (req, res) {
app.get<{ Params: { id: string } }>('/test-inbound-headers/:id', function (req, res) {
const headers = req.headers;

res.send({ headers, id: req.params.id });
});

app.get('/test-outgoing-http/:id', async function (req, res) {
app.get<{ Params: { id: string } }>('/test-outgoing-http/:id', async function (req, res) {
const id = req.params.id;
const data = await makeHttpRequest(`http://localhost:3030/test-inbound-headers/${id}`);

res.send(data);
});

app.get('/test-outgoing-fetch/:id', async function (req, res) {
app.get<{ Params: { id: string } }>('/test-outgoing-fetch/:id', async function (req, res) {
const id = req.params.id;
const response = await fetch(`http://localhost:3030/test-inbound-headers/${id}`);
const data = await response.json();
Expand All @@ -55,7 +68,7 @@ app.get('/test-error', async function (req, res) {
res.send({ exceptionId });
});

app.get('/test-exception/:id', async function (req, res) {
app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) {
throw new Error(`This is an exception with id ${req.params.id}`);
});

Expand Down Expand Up @@ -101,9 +114,9 @@ app2.get('/external-disallowed', function (req, res) {

app2.listen({ port: port2 });

function makeHttpRequest(url) {
function makeHttpRequest(url: string) {
return new Promise(resolve => {
const data = [];
const data: any[] = [];

http
.request(url, httpRes => {
Expand Down

This file was deleted.

8 changes: 5 additions & 3 deletions packages/node/src/integrations/tracing/fastify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ export const fastifyIntegration = defineIntegration(_fastifyIntegration);

// We inline the types we care about here
interface Fastify {
register: (plugin: unknown) => void;
addHook: (hook: string, handler: (request: unknown, reply: unknown, error: Error) => void) => void;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
register: (plugin: any) => void;
Copy link
Member

Choose a reason for hiding this comment

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

Should we make these unknown instead? That forces us to check the things, which I guess is the safer approach? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

If we use unknown the types don't line up, that's why we switched to any

see:
image

// eslint-disable-next-line @typescript-eslint/no-explicit-any
addHook: (hook: string, handler: (request: any, reply: any, error: Error) => void) => void;
}

/**
Expand All @@ -53,7 +55,7 @@ interface FastifyRequestRouteInfo {
*/
export function setupFastifyErrorHandler(fastify: Fastify): void {
const plugin = Object.assign(
function (fastify: Fastify, options: unknown, done: () => void): void {
function (fastify: Fastify, _options: unknown, done: () => void): void {
fastify.addHook('onError', async (_request, _reply, error) => {
captureException(error);
});
Expand Down