fix: client-safe errors reported as exceptions to the exceptions channel

Merged
AB
2mo ago
+108-14

Proposed changes (including videos or screenshots)

When Log_Level is set to 2 (Debug), every method rejection was being posted to the room configured in Log Exceptions to Channel, including expected, client-safe errors. Customers saw their exceptions channel flooded with things like error-invalid-user, wrong passwords from login, permission denials and totp-required, none of which are actual exceptions.

The method.call and method.callAnon endpoints each had two logging blocks, and only the first one filtered expected errors:

if (!err.isClientSafe && !err.meteorError) {
    SystemLogger.error({ msg: 'Exception while invoking method', err, method });  // filtered
}

if (settings.get('Log_Level') === '2') {
    Meteor._debug(`Exception while invoking method ${method}`, err);              // not filtered
}

Since Log_Exceptions_to_Channel monkey patches Meteor._debug, anything passed to it gets posted to the channel, so the unfiltered second block was the leak.

The duplicated logic in both endpoints is now a single logMethodCallError helper in server/api/lib/, which returns early for client-safe and Meteor errors so neither the error log nor Meteor._debug runs. Expected errors are still logged through SystemLogger.debug, which is not intercepted by the channel handler, so no diagnostic visibility is lost. Log_Level: 2 maps to the pino debug level, so these errors surface in the server logs under exactly the same setting as before, just not in the channel.

Genuine exceptions are unaffected and are still reported to the channel.

Issue(s)

SUP-1096

Steps to test or reproduce

  1. Create a channel to receive exceptions, e.g. exceptions-test.
  2. In Admin > Settings > Logs, set Log Exceptions to Channel to exceptions-test and Log Level to 2 - Debug.
  3. Trigger an expected, client-safe error, for example an unauthenticated method call:
    curl -X POST http://localhost:3000/api/v1/method.callAnon/loadHistory \
      -H "Content-Type: application/json" \
      -d '{"message":"{\"msg\":\"method\",\"method\":\"loadHistory\",\"params\":[\"GENERAL\",null,20,null],\"id\":\"1\"}"}'
    
    The response contains error-invalid-user with isClientSafe: true.
  4. Expected result: nothing is posted to exceptions-test. Before this change, an Exception while invoking method loadHistory message was posted.
  5. Trigger a real exception to confirm reporting still works, for example a failing check():
    curl -X POST http://localhost:3000/api/v1/method.call/loadHistory \
      -H "X-Auth-Token: <token>" -H "X-User-Id: <userId>" -H "Content-Type: application/json" \
      -d '{"message":"{\"msg\":\"method\",\"method\":\"loadHistory\",\"params\":[12345,null,20,null],\"id\":\"2\"}"}'
    
  6. Expected result: Exception while invoking method loadHistory is posted to exceptions-test, since a Match error is not client-safe.

Further comments

This covers Task 1 from the refinement session on the ticket, which addresses the log noise. Task 2, the post-logout "zombie request" where getMore keeps recursing after the client's credentials are gone, was split into a separate sub-task and is not addressed here. The loadHistory call will still be made and still rejected after logout, it just no longer pollutes the exceptions channel.

Verified manually against a local server for both endpoints: client-safe errors from method.call and method.callAnon are no longer posted, while a non client-safe Match error still is.

SUP-1096

Summary by CodeRabbit

  • Bug Fixes
    • Client-safe errors are no longer incorrectly reported as exceptions in the configured log channel.
    • Routine client and Meteor errors are now logged at debug level, reducing unnecessary error noise.
    • Unexpected method invocation failures continue to be recorded as errors for troubleshooting.