Skip to content

fix: make GraphError body/cause non-enumerable#552

Merged
heyitsaamir merged 3 commits into
mainfrom
fix/graph-error-clean-output
Apr 30, 2026
Merged

fix: make GraphError body/cause non-enumerable#552
heyitsaamir merged 3 commits into
mainfrom
fix/graph-error-clean-output

Conversation

@heyitsaamir

@heyitsaamir heyitsaamir commented Apr 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

  • body and source (previously cause) on GraphError use Object.defineProperty with enumerable: false
  • cause renamed to source because Node's util.inspect hard-codes [cause] display regardless of enumerability
  • no data lost — err.body, err.source still work for programmatic access

Before

GraphError: Graph GET /me/messages failed (403): Insufficient privileges...
    at Client.call (...) {
  statusCode: 403,
  code: 'Authorization_RequestDenied',
  body: {
    error: {
      code: 'Authorization_RequestDenied',
      message: 'Insufficient privileges to complete the operation.',
      innerError: {
        date: '2026-04-30T22:15:00',
        'request-id': 'a1b2c3d4-...',
        'client-request-id': 'f9e8d7c6-...',
      }
    }
  },
  [cause]: Error: Request failed with status code 403
      at ... {
    isAxiosError: true,
    response: {
      status: 403,
      data: { error: { code: '...', message: '...', innerError: [Object] } }
    },
    config: {
      url: '/me/messages',
      method: 'get',
      headers: { Authorization: 'Bearer eyJ0eXAi...' }
    }
  }
}

After

GraphError: Graph GET /me/messages failed (403): Insufficient privileges...
    at Client.call (...) {
  statusCode: 403,
  code: 'Authorization_RequestDenied'
}

Test plan

  • npx jest in packages/graph — all 64 tests pass
  • throw a GraphError and confirm console output is clean
  • confirm err.body and err.source still return correct values

@heyitsaamir @claude
fix: make GraphError body and cause non-enumerable for cleaner logs
22866f8 
body and cause dumped the full Graph API response + raw axios error
(including config with auth headers) on every console.log/throw.
Now hidden from inspect output but still accessible programmatically.

Co-Authored-By: Claude <[email protected]>
Copilot AI review requested due to automatic review settings April 30, 2026 22:04
Copilot AI reviewed Apr 30, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates GraphError so sensitive/verbose fields (body, cause) are non-enumerable to keep logs and thrown error output concise and reduce accidental exposure of Graph API responses and HTTP client details.

Changes:

  • Stop passing cause via Error constructor options and instead attach cause as a non-enumerable property.
  • Attach body as a non-enumerable property via Object.defineProperty.
  • Make body/cause non-writable to prevent mutation after construction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/graph/src/index.ts
Comment thread packages/graph/src/index.ts Outdated
Comment thread packages/graph/src/index.ts Outdated
Comment thread packages/graph/src/index.ts Outdated
heyitsaamir and others added 2 commits April 30, 2026 15:14
@heyitsaamir @claude
address review: restore native cause semantics, relax descriptors, ad…
569f425 
…d tests

- Use super(message, { cause }) to preserve native Error.cause behavior,
  then redefine enumerability after
- Use writable: true, configurable: true to match native Error semantics
- Add test asserting body/cause are hidden from Object.keys and JSON.stringify

Co-Authored-By: Claude <[email protected]>
@heyitsaamir @claude
fix: drop cause (Node always prints it), use source instead
8b10720 
Node's util.inspect special-cases [cause] and always prints it
regardless of enumerability. Renamed to `source` to avoid this.
No data lost — err.source holds the original axios error.

Co-Authored-By: Claude <[email protected]>
@heyitsaamir
heyitsaamir merged commit 0fa5773 into main Apr 30, 2026
7 checks passed
@heyitsaamir
heyitsaamir deleted the fix/graph-error-clean-output branch April 30, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@Jesperholmbergmsft Jesperholmbergmsft Jesperholmbergmsft approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@heyitsaamir @Jesperholmbergmsft