Do not store context inside structs

[rolznz]

See getAlby/nostr-wallet-connect-next#92 (comment)

[rdmitr]

Looking closer at this issue, I realized the current implementation should probably be left as is.

First of all, the rule of thumb in Go is to pass context as a parameter to methods instead of storing it in structs.

However, in NWC, the context is stored within Service. The intention, apparently, is to make it possible to abort currently running requests when the app is stopped and the parent context is cancelled. Since we have potentially long-running requests (i.e. requests going to the Lightning network), it does make sense. On the other hand, it only seems to be used in LND, because other clients call Rust code through FFI and Go contexts do not work there.

[rolznz]

I think there are still some places where we can reduce the usage of the context stored in the service - by moving the signal.NotifyContext to the entrypoints (main_http and main_wails) and pass it as a parameter further down, rather than creating it in the service. There are some places in the api where we use svc.ctx instead of passing a context in (e.g. the HTTP service could pass the echo request context to the api functions such as ListChannels)

Would that sound like a good idea to you?

[rdmitr]

I think there are still some places where we can reduce the usage of the context stored in the service - by moving the signal.NotifyContext to the entrypoints (main_http and main_wails) and pass it as a parameter further down, rather than creating it in the service

Right, that makes sense. I moved it to main_http. However, I am not so sure about Wails — do we need to handle signals there? Doesn't Wails handle shutdown on its own?

There are some places in the api where we use svc.ctx instead of passing a context in (e.g. the HTTP service could pass the echo request context to the api functions such as ListChannels)

That could work, actually. First I thought it was impossible, since echo.Context is totally not the same as golang's Context. Then I realised that we still can get the request through echo.Context's Request() method, and then get the request's context, which should propagate cancellation when either a client drops connection or the server is shutdown.

I will implement that!

[rdmitr]

I have implemented the proposed changes.

I tried to remove the context from the service entirely, but that turned out to be much trickier. The service's context is used as the parent context when starting the API in the /api/start endpoint. We cannot use request context instead, because it is cancelled when the client connection is closed.