Fix nested field access in message template formatting (#175)

Co-authored-by: ayaangazali <ayaan.gazly@gmail.com>

Author: ayaangazali

.changeset/nested-template-fields.md

@@ -0,0 +1,5 @@
+---
+'logfire': patch
+---
+
+Fix nested field access in message templates. `{a.b}` previously resolved every path segment against the top-level attribute record, so it either fell back to the raw template or silently rendered an unrelated top-level attribute that shared the trailing segment name. Nested paths now walk into the attribute value, matching Python Logfire, and literal dotted attribute keys like `http.method` keep their existing precedence. Field lookups now use `Object.hasOwn`, so prototype members like `{user.toString}` no longer resolve. Index-style bracket syntax such as `{a[0]}` was never supported; when no literal attribute key matches, it previously rendered the string `undefined` and now warns and falls back to the raw template. An attribute whose literal key contains brackets (e.g. `'a[0]': value`) keeps resolving, as it did before.

packages/logfire-api/src/formatter.test.ts

@@ -0,0 +1,78 @@
+import { afterEach, describe, expect, test, vi } from 'vite-plus/test'
+
+import { NoopScrubber } from './AttributeScrubber'
+import { logfireFormatWithExtras } from './formatter'
+
+function format(template: string, record: Record<string, unknown>): string {
+  return logfireFormatWithExtras(template, record, NoopScrubber).formattedMessage
+}
+
+describe('message template nested field access', () => {
+  afterEach(() => {
+    vi.restoreAllMocks()
+  })
+
+  test('resolves a nested field', () => {
+    expect(format('hello {user.name}', { user: { name: 'Alice' } })).toBe('hello Alice')
+  })
+
+  test('resolves a deeply nested field', () => {
+    expect(format('request by {req.user.id}', { req: { user: { id: 1 } } })).toBe('request by 1')
+  })
+
+  test('nested traversal ignores unrelated top-level keys with the same trailing name', () => {
+    expect(format('value is {a.b}', { a: { b: 'nested' }, b: 'top' })).toBe('value is nested')
+  })
+
+  test('a literal dotted attribute key wins over nested traversal', () => {
+    expect(format('{http.method} request', { http: { method: 'nested' }, 'http.method': 'GET' })).toBe('GET request')
+  })
+
+  test('supports the debug format with nested fields', () => {
+    expect(format('{user.name=}', { user: { name: 'Alice' } })).toBe('user.name=Alice')
+  })
+
+  test('falls back to the raw template when the nested field is missing', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
+    expect(format('hello {user.name}', { user: {} })).toBe('hello {user.name}')
+    expect(warn).toHaveBeenCalledWith('Formatting error: The field user.name is not defined.')
+  })
+
+  test('falls back to the raw template when the root of a nested field is missing', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
+    expect(format('hello {user.name}', { other: 1 })).toBe('hello {user.name}')
+    expect(warn).toHaveBeenCalledWith('Formatting error: The field user.name is not defined.')
+  })
+
+  test('falls back to the raw template when an intermediate value is not an object', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
+    expect(format('{a.b}', { a: 5 })).toBe('{a.b}')
+    expect(warn).toHaveBeenCalledWith('Formatting error: The field a.b is not defined.')
+  })
+
+  test('plain top-level fields keep working', () => {
+    expect(format('hello {name}', { name: 'Bob' })).toBe('hello Bob')
+  })
+
+  test('nested traversal does not resolve prototype members', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
+    expect(format('{user.toString}', { user: {} })).toBe('{user.toString}')
+    expect(warn).toHaveBeenCalledWith('Formatting error: The field user.toString is not defined.')
+  })
+
+  test('top-level lookup does not resolve prototype members', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
+    expect(format('{toString}', { name: 'Bob' })).toBe('{toString}')
+    expect(warn).toHaveBeenCalledWith('Formatting error: The field toString is not defined.')
+  })
+
+  test('bracket syntax with no matching literal key falls back to the raw template', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
+    expect(format('first item is {a[0]}', { a: ['zero'] })).toBe('first item is {a[0]}')
+    expect(warn).toHaveBeenCalledWith('Formatting error: The field a[0] is not defined.')
+  })
+
+  test('a literal attribute key containing brackets keeps resolving', () => {
+    expect(format('first item is {a[0]}', { 'a[0]': 'zero' })).toBe('first item is zero')
+  })
+})

packages/logfire-api/src/formatter.ts

@@ -56,35 +56,33 @@ class ChunksFormatter {
 
   // Equivalent to Python's getField method
   getField(fieldName: string, record: Record<string, unknown>): [unknown, string] {
-    if (fieldName.includes('.') || fieldName.includes('[')) {
-      // Handle nested field access like "a.b" or "a[b]"
-      try {
-        // Simple nested property access (this is a simplification)
-        const parts = fieldName.split('.')
-        let obj = record[parts[0] ?? '']
-        for (let i = 1; i < parts.length; i++) {
-          const key = parts[i] ?? ''
-          if (key in record) {
-            obj = record[key]
-          } else {
-            throw new KnownFormattingError(`The field ${fieldName} is not an object.`)
-          }
-        }
-        return [obj, parts[0] ?? '']
-      } catch {
-        // Try getting the whole thing from object
-        if (fieldName in record) {
-          return [record[fieldName], fieldName]
-        }
+    // A literal attribute key always wins, so OTel-style dotted names like
+    // "http.method" keep resolving even when nested traversal is possible.
+    // Object.hasOwn keeps prototype members like `toString` from resolving.
+    if (Object.hasOwn(record, fieldName)) {
+      return [record[fieldName], fieldName]
+    }
+
+    if (fieldName.includes('.')) {
+      // Handle nested field access like "a.b" by walking into the record value
+      const parts = fieldName.split('.')
+      const firstKey = parts[0] ?? ''
+      if (!Object.hasOwn(record, firstKey)) {
         throw new KnownFormattingError(`The field ${fieldName} is not defined.`)
       }
-    } else {
-      // Simple field access
-      if (fieldName in record) {
-        return [record[fieldName], fieldName]
+      let obj: unknown = record[firstKey]
+      for (let i = 1; i < parts.length; i++) {
+        const key = parts[i] ?? ''
+        if (typeof obj === 'object' && obj !== null && Object.hasOwn(obj, key)) {
+          obj = (obj as Record<string, unknown>)[key]
+        } else {
+          throw new KnownFormattingError(`The field ${fieldName} is not defined.`)
+        }
       }
-      throw new KnownFormattingError(`The field ${fieldName} is not defined.`)
+      return [obj, firstKey]
     }
+
+    throw new KnownFormattingError(`The field ${fieldName} is not defined.`)
   }
 
   parse(formatString: string): [string, null | string, null | string, null | string][] {