OpenFn
Send OAuth2 refresh errors to the logger
Fixes #2135
This is the lightning side of https://github.com/OpenFn/lightning/issues/1842#issuecomment-2110272907
Questions about how we make this safe... both in terms of "is it a string" (see my simplistic inspect) and in terms of "how do we ensure we don't expose things we don't want to expose"?
@elias-ba , please take a look here — this is what's required to debug credential errors in the log.
@christad92 what's the status of this guy? this was part of Elias' Oauth Client epic and I'm afraid it's been forgotten!
The only place I can see an error happening that the worker wouldn't be aware of is when we maybe refresh the credential before sending the job to the worker. So what we should do is narrow down the issue / work to do to that and talk to Joe to define an interface that would allow us to pass to the worker an error representing a failed to refresh credential instead of an actual rotten credential. And the worker would display that error in the logs and wouldn't even attempt to use the credential. Other than that I think any other error would be handled by the worker.
So for actions to take on this one, I would suggest, I sit with @josephjclark when his available and define an interface and I implement the Lightning side of it, which should be really easy (maybe I would even be able to reuse parts of this PR).
What do you guys think ? cc @christad92 @taylordowns2000 @stuartc
The interface was defined and implemented in [email protected]. If there are things you want to change in this PR you are welcome to, but I'd start by using it as is. I think you mentioned that you wanted to add some new stuff after your final round of updates to the refresh flow.
More notes from issue: https://github.com/OpenFn/lightning/issues/1842#issuecomment-2110272907
On Wed, Jul 10, 2024 at 10:55 AM Elias W. BA @.***> wrote:
The only place I can see an error happening that the worker wouldn't be aware of is when we maybe refresh the credential before sending the job to the worker. So what we should do is narrow down the issue / work to do to that and talk to Joe to define an interface that would allow us to pass to the worker an error representing a failed to refresh credential instead of an actual rotten credential. And the worker would display that error in the logs and wouldn't even attempt to use the credential. Other than that I think any other error would be handled by the worker.
So for actions to take on this one, I would suggest, I sit with @josephjclark https://github.com/josephjclark when his available and define an interface and I implement the Lightning side of it, which should be really easy (maybe I would even be able to reuse parts of this PR).
What do you guys think ? cc @christad92 https://github.com/christad92 @taylordowns2000 https://github.com/taylordowns2000 @stuartc https://github.com/stuartc
— Reply to this email directly, view it on GitHub https://github.com/OpenFn/lightning/pull/2086#issuecomment-2220060567, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCUBLJWRP3FAE5L37GVS3TZLUAHTAVCNFSM6AAAAABHWS2S62VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRQGA3DANJWG4 . You are receiving this because you were mentioned.Message ID: @.***>
-- Taylor Downs CEO, OpenFn https://www.openfn.org
@elias-ba when reviewing, please focus on (1) sensitive data we may send to the worker (2) does this pr cover changes brought in later (like refresh flow).
@elias-ba what is happening to this PR. Should we keep it open or it should be closed?
@elias-ba what's up on this pr? neither Ayo or I can quite tell why it was closed and then reopened. is this code (or something similar) already in main?
Closing now as the intent of this code was implemented in #2289 in July. It was reopened accidentally.