[PR] Do not return NotImplementedError messages to the caller [pekko-grpc]

["pjfanning (via GitHub)" <gi...@apache.org>]

pjfanning opened a new pull request, #874:
URL: https://github.com/apache/pekko-grpc/pull/874

   ### Motivation
   
   `defaultMapper` logs and scrubs the message of every failure it does not
   recognise, but two cases were different:
   
   ```scala
   case e: NotImplementedError           => Trailers(Status.UNIMPLEMENTED.withDescription(e.getMessage))
   case e: UnsupportedOperationException => Trailers(Status.UNIMPLEMENTED.withDescription(e.getMessage))
   ```
   
   Those messages come from whatever the service implementation threw, so an
   exception raised deep inside a handler put its text on the wire. Neither case was
   logged either, so the detail went to the caller and nowhere else — the opposite of
   every other branch, which logs and returns a bare status.
   
   `???` in Scala throws `NotImplementedError`, and `UnsupportedOperationException`
   comes out of many collection and stream operations, so these are easy to hit
   without intending to report anything.
   
   ### Modification
   
   Both cases now log at warning and answer with a bare `UNIMPLEMENTED`, matching how
   the mapper treats every other unrecognised failure — in both the scaladsl and
   javadsl handlers.
   
   `GrpcServiceException` remains the way to send a description deliberately, so the
   generated handlers now raise that for an unknown method rather than relying on the
   message of a `NotImplementedError` reaching the caller. **The behaviour a client
   sees for an unknown method is unchanged.**
   
   ### Result
   
   An accidental `???` or unsupported collection operation no longer describes itself
   to the caller, and the detail is logged where an operator can see it. Deliberate
   descriptions still work.
   
   ### Tests
   
   - 3 new cases in `GrpcExceptionHandlerSpec`: the message of each exception type is
     withheld, and a deliberate `GrpcServiceException` description still arrives
   - The existing table-driven expectation is updated — that diff *is* the behaviour
     change
   - The existing end-to-end `ErrorReportingSpec` in both plugin-testers already
     asserts that a client calling an unknown method receives
     `grpc-message: Not implemented: UnknownMethod`. Both still pass, which is what
     covers the codegen half of this change
   
   Confirmed the guard bites: restoring the leak fails 3 tests.
   
   - `sbt "runtime/testOnly ...GrpcExceptionHandlerSpec"` — 29 passed
   - `sbt "plugin-tester-scala/testOnly ...ErrorReportingSpec"` — 2 passed
   - `sbt "plugin-tester-java/testOnly ...ErrorReportingSpec"` — 2 passed
   - `sbt "runtime/mimaReportBinaryIssues"` — passed
   - `sbt scalafmtAll scalafmtSbt` — applied
   - `sbt "runtime/test"` (full suite) — not run locally, left to CI
   
   ### Notes
   
   A service that was relying on a thrown `NotImplementedError`'s message reaching
   its callers will now see an empty description. That is the point of the change,
   but it is a visible difference: the fix is to throw a `GrpcServiceException` with
   the intended description, which is what the generated handlers now do.
   
   Handlers generated before this change still throw `NotImplementedError` for an
   unknown method and so will report no description until regenerated — the same
   regeneration caveat #818 carries.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org

Re: [PR] Do not return NotImplementedError messages to the caller [pekko-grpc]

["pjfanning (via GitHub)" <gi...@apache.org>]

pjfanning merged PR #874:
URL: https://github.com/apache/pekko-grpc/pull/874


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org