Merge pull request #166 from github-community-projects/sutterj/revamp…

…-tslog

feat: update tslog

src/app/api/auth/lib/nextauth-options.ts

@@ -118,16 +118,16 @@ export const nextAuthOptions: AuthOptions = {
       if (!(metadata instanceof Error) && metadata.provider) {
         // redact the provider secret here
         delete metadata.provider
-        authLogger.error({ code, metadata })
+        authLogger.error('Auth error', { code, metadata })
       } else {
-        authLogger.error({ code, metadata })
+        authLogger.error('Auth error', { code, metadata })
       }
     },
     warn(code) {
-      authLogger.warn({ code })
+      authLogger.warn('Auth warn', { code })
     },
     debug(code, metadata) {
-      authLogger.debug({ code, metadata })
+      authLogger.debug('Auth debug', { code, metadata })
     },
   },
   callbacks: {

src/app/context/AuthProvider.tsx

@@ -5,6 +5,9 @@ import { Session } from 'next-auth'
 import { SessionProvider, signOut, useSession } from 'next-auth/react'
 
 import { ReactNode, useEffect } from 'react'
+import { logger } from 'utils/logger'
+
+const authProviderLogger = logger.getSubLogger({ name: 'auth-provider' })
 
 const VerifiedAuthProvider = ({ children }: { children: ReactNode }) => {
   const session = useSession()
@@ -16,12 +19,12 @@ const VerifiedAuthProvider = ({ children }: { children: ReactNode }) => {
     }
 
     if (session.data?.error === 'RefreshAccessTokenError') {
-      console.error('Could not refresh access token - signing out')
+      authProviderLogger.error('Could not refresh access token - signing out')
       signOut()
     }
 
     if (session.data && new Date(session.data.expires) < new Date()) {
-      console.log('session expired - signing out')
+      authProviderLogger.info('session expired - signing out')
       signOut()
     }
   }, [

src/bot/config.ts

@@ -55,9 +55,8 @@ export const getEnvConfig = () => {
 export const validateConfig = (config: InternalContributionForksConfig) => {
   try {
     internalContributionForksConfig.parse(config)
-  } catch (e) {
-    configLogger.error('Invalid config found!')
-    configLogger.error(e)
+  } catch (error) {
+    configLogger.error('Invalid config found!', { error })
     throw new Error(
       'Invalid config found! Please check the config and error log for more details.',
     )
@@ -82,15 +81,15 @@ export const getConfig = async (orgId?: string) => {
 
   // Lastly check github for a config
   if (!orgId) {
-    logger.error(
+    configLogger.error(
       'No orgId present, Organization ID is required to set a config when not using environment variables',
     )
     throw new Error('Organization ID is required to set a config!')
   }
 
   config = await getGitHubConfig(orgId)
 
-  logger.info(`Using following config values`, {
+  configLogger.info(`Using following config values`, {
     config,
   })
 

src/bot/rules.ts

@@ -25,13 +25,13 @@ export const createAllPushProtection = async (
   repositoryNodeId: string,
   actorNodeId: string,
 ) => {
-  rulesLogger.debug('Creating branch protection ruleset for fork', {
+  rulesLogger.info('Creating branch protection for all branches', {
     repositoryOwner: context.payload.repository.owner.login,
     repositoryName: context.payload.repository.name,
   })
 
   try {
-    // Add branch protection via rulesets to the all branches
+    // Add branch protection via rulesets to all branches
     await createBranchProtectionRuleset(
       context,
       actorNodeId,
@@ -40,22 +40,35 @@ export const createAllPushProtection = async (
     )
   } catch (error) {
     rulesLogger.error(
-      'Failed to create branch protection for fork, falling back to branch protections',
+      'Failed to create branch protection via rulesets, falling back to branch protections',
       {
         error,
       },
     )
 
-    const createBranchProtectionRes = await createBranchProtection(
-      context,
-      repositoryNodeId,
-      '*',
-      actorNodeId,
-    )
+    try {
+      // Add branch protection via GQL to all branches
+      await createBranchProtection(context, repositoryNodeId, '*', actorNodeId)
+    } catch (error) {
+      rulesLogger.error(
+        'Failed to create branch protection via GQL, falling back to REST',
+        {
+          error,
+        },
+      )
 
-    rulesLogger.info('Branch protection created', {
-      createBranchProtectionRes,
-    })
+      try {
+        // Add branch protection via REST to all branches
+        await createBranchProtectionREST(context, '*')
+      } catch (error) {
+        rulesLogger.error(
+          'Failed to create branch protection for all branches',
+          {
+            error,
+          },
+        )
+      }
+    }
   }
 }
 
@@ -73,13 +86,13 @@ export const createDefaultBranchProtection = async (
   actorNodeId: string,
   defaultBranch: string,
 ) => {
-  rulesLogger.debug('Creating branch protection ruleset for mirror', {
+  rulesLogger.info('Creating branch protection for default branch', {
     repositoryOwner: context.payload.repository.owner.login,
     repositoryName: context.payload.repository.name,
   })
 
   try {
-    // Add branch protections via ruleset to the default branch
+    // Add branch protection via ruleset to the default branch
     await createBranchProtectionRuleset(
       context,
       actorNodeId,
@@ -96,17 +109,14 @@ export const createDefaultBranchProtection = async (
     )
 
     try {
-      const createBranchProtectionRes = await createBranchProtection(
+      // Add branch protection via GQL to the default branch
+      await createBranchProtection(
         context,
         repositoryNodeId,
         defaultBranch,
         actorNodeId,
         true,
       )
-
-      rulesLogger.info('Branch protection created', {
-        createBranchProtectionRes,
-      })
     } catch (error) {
       rulesLogger.error(
         'Failed to create branch protection from BP GQL, trying REST instead',
@@ -115,13 +125,17 @@ export const createDefaultBranchProtection = async (
         },
       )
 
-      const createBranchProtectionRes = await createBranchProtectionREST(
-        context,
-        defaultBranch,
-      )
-      rulesLogger.info('Branch protection created', {
-        createBranchProtectionRes,
-      })
+      try {
+        // Add branch protection via REST to the default branch
+        await createBranchProtectionREST(context, defaultBranch)
+      } catch (error) {
+        rulesLogger.error(
+          'Failed to create branch protection for default branch',
+          {
+            error,
+          },
+        )
+      }
     }
   }
 }
@@ -140,6 +154,10 @@ const createBranchProtectionRuleset = async (
   includeRefs: string[],
   isMirror = false,
 ) => {
+  rulesLogger.info('Creating branch protection via rulesets', {
+    isMirror,
+  })
+
   // Get the current branch protection rulesets
   const getBranchProtectionRuleset = await context.octokit.graphql<{
     repository: Repository
@@ -153,7 +171,7 @@ const createBranchProtectionRuleset = async (
       (ruleset) => ruleset?.name === ruleName,
     )
   ) {
-    rulesLogger.info('Branch protection rule already exists', {
+    rulesLogger.info('Branch protection ruleset already exists', {
       getBranchProtectionRuleset,
     })
 
@@ -172,7 +190,7 @@ const createBranchProtectionRuleset = async (
     includeRefs,
   })
 
-  rulesLogger.info('Created branch protection rule', {
+  rulesLogger.info('Created branch protection via rulesets', {
     branchProtectionRuleset,
   })
 }
@@ -191,7 +209,7 @@ const createBranchProtection = async (
   actorId: string,
   isMirror = false,
 ) => {
-  rulesLogger.info('Creating branch protection', {
+  rulesLogger.info('Creating branch protection via GQL', {
     isMirror,
   })
 
@@ -203,7 +221,7 @@ const createBranchProtection = async (
     actorId,
   })
 
-  rulesLogger.info('Created branch protection', {
+  rulesLogger.info('Created branch protection via GQL', {
     forkBranchProtection,
   })
 }
@@ -217,6 +235,8 @@ const createBranchProtectionREST = async (
   context: ContextEvent,
   pattern: string,
 ) => {
+  rulesLogger.info('Creating branch protection via REST')
+
   const res = await context.octokit.repos.updateBranchProtection({
     branch: pattern,
     enforce_admins: true,
@@ -235,7 +255,7 @@ const createBranchProtectionREST = async (
     restrictions: null,
   })
 
-  rulesLogger.info('Created branch protection rule to default branch', {
+  rulesLogger.info('Created branch protection via REST', {
     res,
     repositoryOwner: context.payload.repository.owner.login,
     repositoryName: context.payload.repository.name,

src/server/octokit/controller.ts

@@ -23,7 +23,7 @@ export const checkInstallationHandler = async ({
 
     return { installed: false }
   } catch (error) {
-    octokitApiLogger.info('Failed to check installation', { input, error })
+    octokitApiLogger.error('Failed to check app installation', { input, error })
 
     return { installed: false }
   }

src/server/repos/controller.ts

@@ -23,7 +23,7 @@ export const createMirrorHandler = async ({
 
     const config = await getConfig(input.orgId)
 
-    reposApiLogger.debug('Fetched config', config)
+    reposApiLogger.debug('Fetched config', { config })
 
     const { publicOrg, privateOrg } = config
 
@@ -52,15 +52,15 @@ export const createMirrorHandler = async ({
           `Repo ${orgData.data.login}/${input.newRepoName} already exists`,
         )
       }
-    } catch (e) {
+    } catch (error) {
       // We just threw this error, so we know it's safe to rethrow
-      if ((e as Error).message.includes('already exists')) {
-        throw e
+      if ((error as Error).message.includes('already exists')) {
+        throw error
       }
 
-      if (!(e as Error).message.includes('Not Found')) {
-        logger.error({ error: e })
-        throw e
+      if (!(error as Error).message.includes('Not Found')) {
+        reposApiLogger.error('Not found', { error })
+        throw error
       }
     }
 
@@ -140,16 +140,16 @@ export const createMirrorHandler = async ({
         success: true,
         data: newRepo.data,
       }
-    } catch (e) {
+    } catch (error) {
       // Clean up the private mirror repo made
       await privateOctokit.rest.repos.delete({
         owner: privateOrg,
         repo: input.newRepoName,
       })
 
-      logger.error({ error: e })
+      reposApiLogger.error('Error creating mirror', { error })
 
-      throw e
+      throw error
     }
   } catch (error) {
     reposApiLogger.error('Error creating mirror', { error })