Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPSP link events (load/error more than once? loadstart/abort?) #239

Open
sublimator opened this issue Dec 5, 2021 · 22 comments
Open

SPSP link events (load/error more than once? loadstart/abort?) #239

sublimator opened this issue Dec 5, 2021 · 22 comments
Labels
deep dive In-depth discussions on related topics. Should spawn more specific issues. Use for historical record specification Work required on specification

Comments

@sublimator
Copy link
Collaborator

So I was adding in the routing of SPSP events in the morning and recalled that we retry (*) on 5xx and abort on 4xx

At least in the extension now, if the STREAM gets broken (say due to bad net) then the SPSP will be reloaded before reconnecting.

So with the extension now a link tag could emit events for more than one of: load and/or error

Also, would loadstart / abort events be suitable here ?

  • At some point I could have sworn we retried up to a certain amount of times (say 5) - though I think in main (and the extension that is currently published) we actually just keep retrying, albeit with a 2 second delay.
@sublimator
Copy link
Collaborator Author

Related issue/discussion
#149

@marcoscaceres marcoscaceres changed the title WM2: SPSP link events (load/error more than once? loadstart/abort?) SPSP link events (load/error more than once? loadstart/abort?) Dec 6, 2021
@marcoscaceres
Copy link
Contributor

marcoscaceres commented Dec 6, 2021

So with the extension now a link tag could emit events for more than one of: load and/or error

That's correct. That should hopefully be clear in the updated spec... basically, when the href attribute changes, or other things defined in the spec cause the JSON for the SPSP to download, then load/error fire again (based on what happens).

Also, would loadstart / abort events be suitable here ?

I don't think so (loadstart is for media stuff, I think... and "legacy" XHR)... but happy to discuss.

At some point I could have sworn we retried up to a certain amount of times (say 5) - though I think in main (and the extension that is currently published) we actually just keep retrying, albeit with a 2 second delay.

The load should fire after the JSON is downloaded and parsed... SPSP and STREAM can then retry independently after.

@sublimator
Copy link
Collaborator Author

sublimator commented Dec 6, 2021 via email

@sublimator
Copy link
Collaborator Author

sublimator commented Dec 6, 2021 via email

@marcoscaceres
Copy link
Contributor

marcoscaceres commented Dec 6, 2021

Note that downloading the JSON is the only part of SPSP that uses the
payment pointer endpoint. The rest is STREAM setup using the details
encoded in the JSON.

I think am I not understanding you.

No problem. Let's try this again ☺️

The only part the spec is currently concerned about:

  1. Is there a resource at the end of href= URL? If the Response is "not OK" (HTTP 404, 500, etc), fire error event and stop.
  2. Does the response have the expected JSON MIME type? In no, fire error event and stop.
  3. Can we JSON.parse() the body of the response? In no, fire error event and stop.
  4. Does the JSON have the expected SPSP properties and are properties of the valid/expected types? If no, fire error event and stop.
  5. Finally, we have everything we need to initiate SPSP: fire load event.

Once SPSP/STREAM take over, no more "load" or "error" events are fired at the link. Only "monetization" events are fired.

Hope that makes more sense.

@sublimator
Copy link
Collaborator Author

sublimator commented Dec 7, 2021 via email

@marcoscaceres
Copy link
Contributor

Right, but what about retries/reconnects ?

These shouldn’t be observable by the web page.

Just use the same SPSP connection details for the life of the page ?

Yes. The assumption being that JSON details found initially are governing what’s happening with respect to WM.

However, if there is an unrecoverable error that happens at the STREAM layer, and the only mitigation/recovery strategy would be for the developer to provide a new payment pointer, then we should consider firing an error.

When speaking to @AlexLakatos, we didn’t come up with such a scenario - but maybe we missed something? When could such a situation occur with STREAM?

@sublimator
Copy link
Collaborator Author

sublimator commented Dec 7, 2021

I guess my concern is it needs to work, not just be an elegant design and that we likely do things for reasons. We retry now cause it provides a better experience in the face of dodgy network and temporary/sporadic service outages etc.

Finally, we have everything we need to initiate SPSP: fire load event.
Doing the GET request to the payment pointer is arguably the SPSP initiation phase ?

we didn’t come up with such a scenario - but maybe we missed something?

Firstly, I was not around for the development of ILP or WM. I was at ripple for a stint before (2013-2016 ish), but didn't really have much to do with ILP. Secondly, I joined Coil when they'd already developed WM. All that to say I'm a bit foggy on why some things are done the way they are.

However, there was some discussion on this over at the issue I linked ( #149) See in particular Ben's (@sharafian)comment:
"Right now we re-fetch the ILP address whenever we use a new privacy token so that privacy tokens are never associated by sending to the same ILP address."

You could just say that's a Coil implementation concern and it sure makes things simpler/easier to just have one load/error event per "new" link.

Actually, you are proposing in your final comment a situation where it could fire a load event, then when the SPSP connection details becomes stale, it would fire an "error" event. To recover from this, user land/integration code would need to inject a new link (toggle disable??) to "retry" ? (may not be so bad??) You would need more detailed "error" events, or just infer this state from having seen the "loaded" already ? (Actually, I was wondering about the shape of the error events, would they contain some kind of details ? request failed ? mime wrong ? not parseable ? or just new Event('error') ?)

As said before, I'm not really familiar with ILP in depth, but I imagine that it's possible that there could be stateful/expiring implementations of SPSP, such that the addresses/secrets don't work forever. Or maybe someone updated their payment pointer to point to some other address? (edit: I'm definitely foggy on how that works, so big grain of salt) Shouldn't the SPSP server's cache headers be respected ?

Sorry if I sound confused, trying to muddle my way through this :)

@sublimator
Copy link
Collaborator Author

Also, I wonder about the receipt nonces

If we use the same receipt nonce for more than one STREAM connection (as would be in the case of this new model where we don't re-request SPSP details before each STREAM connection ? ) could that cause some havoc with balance tracking ? @wilsonianb @sharafian

https://github.com/interledgerjs/ilp-protocol-stream/blob/master/src/server.ts#L164

@sublimator
Copy link
Collaborator Author

sublimator commented Dec 7, 2021

To clarify on the previous comment:

  1. The verifier which middle mans itself between the recipient and the sender is in charge of setting the receipt nonce. It's essentially an SPSP proxy, adding in receipt nonce/secret details in the HTTP headers.
  2. The receipt nonce, along with the stream id (which will generally be the same each connection, see here) is generally used to identify a connection, in order to track a balance
  3. Depending on the algorithm, this could mean that some receipts when submitted are discarded as stale or resubmits (I've seen backends implemented by keeping the "greatest" receipt for a given nonce (edit: sans id, just nonce) /stream id identifying pair)
  4. If that's so, then not re-requesting SPSP before STREAM reconnection could have unintended consequences for balance tracking.

It seems it's in the name:
n - once ?

@wilsonianb
Copy link
Collaborator

Yeah, I think it would be problematic to try to re-use the same nonce + stream id for receipts.

fwiw the SPSP spec says:

If the STREAM connection is closed the Client SHOULD attempt to reconnect with the same parameters or it SHOULD query the Server again for new connection parameters once the time indicated in the Cache-Control header has passed.

and the STREAM spec:

Client streams MUST be odd-numbered starting with 1 and server-initiated streams MUST be even-numbered starting with 2. If an endpoint sends a packet for an unopened stream with the wrong number, the receiving endpoint MUST close the connection with a ProtocolViolationError.

Assuming stream id's reset on reconnections, maybe the client could fast forward to an unused stream id without violating protocol?

@marcoscaceres
Copy link
Contributor

marcoscaceres commented Dec 8, 2021

@wilsonianb wrote:

Yeah, I think it would be problematic to try to re-use the same nonce + stream id for receipts.

Yes, that would be bad. Definitely don't want to do that.

Sorry, I think we are accidentally talking past each other - but the info about how SPSP and STREAM work is really helpful. Let's see if we can get a shared understanding.

The new spec doesn't seek to modify or change the way SPSP or STREAM works. Instead, it's only concerned that:

<link 
   rel="monetization" 
   href="https://example.com/spsp.json"
   onload="SPSP_JSON_Loaded()"
   onerror="SPSP_or_STREAM_Error()"
>

The onload is only concerned with href= spsp.json being fetched and processed to do the setup (and then handed off to the WM agent). Auto-refreshing after cache has expired could happen silently, unless there was a use case to signal that a reload has happened.

The onerror is only concerned with href= spsp.json failing to fetched, or failing to process (invalid JSON).

However, if one of the refreshes errors (ProtocolViolationError), we could emit an Error event after the connection is closed.

Thoughts?

@sublimator
Copy link
Collaborator Author

sublimator commented Dec 8, 2021

So here's the cases for doing more than one SPSP request per unique href

  • Keeps receipts working without faffing with a littering of stream ids
    • not sure how STREAM works, can you just "advance" to an arbitrary stream id?
    • I can't see any API for explicitly setting a stream id using ilp-protocol-stream at least
  • Network resiliency - if using receipts or the SPSP cache control settings demand another actual request

unless there was a use case to signal that a reload has happened.

I think transparency about what's happening isn't necessarily a bad thing? And use cases tend to take care of them selves.

It doesn't seem like we are going to be able to present a nice simple one load OR one error programming model, like is probably the case with say an <img> tag? Am I confused in thinking that's what we are trying to do?

(edit: hit send accidentally before finishing this, so bear with me)

@sublimator
Copy link
Collaborator Author

I'm wondering if a <link> tag is really what we want after all, if we have to keep to link events/semantics etc so closely ?

@sublimator
Copy link
Collaborator Author

I think I'm getting confused and forgetting we are trying to move away from exposing protocol specific state in the API. The link is just a "link" to an SPSP document. It happens to emit monetization events but those are supposed to be from anywhere, right?

@marcoscaceres
Copy link
Contributor

So here's the cases for doing more than one SPSP request per unique href

Just being clear: after the first "load", SPSP can do as many request on itself as it likes. Just "load" should only fire for the first one.

I think transparency about what's happening isn't necessarily a bad thing? And use cases tend to take care of them selves.

Ok, so... that generally that's not how we do add stuff to web APIs: Everything should have a very clear use case or reason for being there.

I understand that "use cases tend to take care of them selves", but those are usually in the form of privacy and security attacks. We should be looking at exposing as little as humanly possible to meet well defined use cases.

It doesn't seem like we are going to be able to present a nice simple one load OR one error programming model, like is probably the case with say an <img> tag? Am I confused in thinking that's what we are trying to do?

One us is confused 😝 But what you describe above is the goal: one single "load" and/or a single "error". I think that's what we have. We might need to have a call tho, as we might be going in circles.

I'm wondering if a <link> tag is really what we want after all, if we have to keep to link events/semantics etc so closely ?

I'm not seeing any semantic violation. I think maybe you are be overthinking this? (or I'm not getting something obvious!).

But - our goal is the same: one "load" event. And, if needed, one "error" event.

@sublimator
Copy link
Collaborator Author

Just use the same SPSP connection details for the life of the page ?
Yes.

You can understand my confusion going from the above to below lol

Just being clear: after the first "load", SPSP can do as many request on itself as it likes. Just "load" should only fire for the first one.

So these are events are more like an initial SPSP validation phase ?

Actually when you mention WM Agent, it's still not clear how that's going to be architected, and what information is shared. Will they have access to the payment pointer to request SPSP again, or will they have some kind of refresh mechanism (as per Sid's version) ?

@sublimator
Copy link
Collaborator Author

Everything should have a very clear use case or reason for being there.

Fair enough. What is the load event for ?
Could we get away with just an error event?

@marcoscaceres
Copy link
Contributor

marcoscaceres commented Dec 8, 2021

Fair enough. What is the load event for ?

for developers, it answers: "is web monetization supported? Can I maybe expect 'monetization' events?"

so, for example:

<script>
// only set up listeners if it loads...
function setupListeners(target) {
     const elem = target.closest(".monetizable")
     elem?.addEventListaner("monetization", makeMeMoney);
}
</script>
<article class="monetizable">
  <link rel="monetization" onload="setupListeners(this)">
</article>

Could we get away with just an error event?

I think the "load" case above is still valid.

@sublimator
Copy link
Collaborator Author

Can we open an issue to discuss WM Provider configuration/implementation ? Does that need to be specified ?

@marcoscaceres
Copy link
Contributor

Can we open an issue to discuss WM Provider configuration/implementation?

If you feel it will have an impact on the shape of the current spec, then absolutely!

Does that need to be specified ?

We can definitely talk about it (or include a note or something)... is this similar to #133?

@sublimator
Copy link
Collaborator Author

for developers, it answers: "is web monetization supported?

Would that only answer that question while WM is in its infancy (impl by Coil extension for example ) ?
In the case of WM impl in the browser, it would only try loading the link if a provider is configured?
If it loaded it routinely it would become less meaningful over time.

If you feel it will have an impact on the shape of the current spec, then absolutely!
It may do

Yeah, #133

I see @AlexLakatos' model (recent comment from December 3rd ish) is a bit different to Sids, though they both rely upon extensions, rather than some web protocol.

Sid's model might be an easier sell in terms of impl burden for browsers. I wonder also if there is more flexibility if the provider has complete control of the STREAMs and where they are initiated from (maybe not even in the browser)

Another consideration is that STREAM doesn't have a hard dependency on BTP (that I know of), yet in this model it would.

@huijing huijing added deep dive In-depth discussions on related topics. Should spawn more specific issues. Use for historical record specification Work required on specification labels Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deep dive In-depth discussions on related topics. Should spawn more specific issues. Use for historical record specification Work required on specification
Projects
None yet
Development

No branches or pull requests

4 participants