@BryanB I’m only learning myself but happy to look at your Promises code if you think it might help.
@happybeing I appreciate that. I think my mistake is trying to nest promises within next(), it seems to go wrong when I do it. The code @loziniak made really helps linearise the promise chain, which is why I’m a fan. I already have example 1 from examples.html of safe_examples using the safe_api.js linked above, and I’m making steady progress on getting example 2 working.
Generally I understand Promises. I’ve worked with them a bit. There are just some subtle things I bump into, like nesting versus chaning. I like to nest Promises to work with external APIs, next()'ing their return values with my own to avoid global scoped variables. Unfortunately I cannot figure out how to combine nesting and chaining sensibly, so I’m just going with some stateful handle stores and linear Promise chain thanks to the safe_api.js above.
1 Like
Okay, so I tried to use your safe_api.js to reconstruct Example 2 from SAFE Tutorial Examples
The biggest issue I ran into was Promise forking. Here’s what happened to me: My App Promise chain ran through from start to finish. Just before app.free(), I ran a function that creates a new MutableData, which forks this.handlePromise from App into this.handlePromise of the MutableData. Before the MutableData.then() contents could finish running, the App’s promise chain went right on into free() because they were not waiting on the MutableData chain. By the time the MutableData chain got around to checking on the data I pushed (strangely without error), the App was freed! Bing bang boom, it’s dead Jim.
So I made a couple modifications. App and App-descendants will have separate super classes. App-descendents will operate then() on this.parent.handlePromise, while App will operate then() on this.handlePromise. In this way, forks are avoided. I also tried to reduce some of the code complexity so that you can call this.then() instead of calling this.handlePromise = this.handlePromise.then().
I’ll submit an issue and PR on GitHub. I didn’t update all the other code to fit the new paradigm, I only updated what I needed. Hopefully it helps. There is still going to be a question of calling the right API bits in the right order, but man this Promise stuff has to be done just right.
It will need tweaking to get your example working, but it got my example working (as detailed in the Issue);
Hi. Great, thanks for a good job. Seems like the wrapper is starting to have live on it’s own 
Maybe we should start a new thread on this project?
I have little time to dive into your code, but the forking issue seems a good one. Maybe freeing the app should wait for all forked promises to resolve? Or maybe we shouldn’t free the app at all, since it will be freed when the window is closed… Just a few alternative ideas to think about.
I was trying to keep function names and logic as close to original API as possible, to avoid confusion for people familiar with it. Perhaps instead of introducing new higher-level methods, we could stay at low-level and just add another abstraction as another wrapper? We could move things like authoriseAndConnect there, since it’s already a generalisation at higher level. What do you think?
Maybe freeing the app should wait for all forked promises to resolve?
Part of my additional code added free() for the App and fixed up free() for the MutableData. One issue with Promises is that a catch() is sort of a conditional branch that, once dealt with, will return to the following next(). Because of this, and other reasons, I don’t think we can rely on e.g. .then((mdHandle) => {}) to reliably provide the mdHandle unless it is directly the result of generating the mdHandle in the first place. So I’ve updated the code to store the App handle into App and the mdHandle into MutableData as stateful information. This is handy (I would argue necessary) for calling free() properly.
I’m not sure free() needs to be forced, it just needs to be supported. It works great in my PR. 
Perhaps instead of introducing new higher-level methods, we could stay at low-level and just add another abstraction as another wrapper?
I’m not opposed. Effectively the low level wrapper would manage the Promise chain and keep stateful information like handles, and otherwise add very little other value. The higher level wrapper would operate over multiple low level calls to perform fairly atomic interactions, like saving a new key to an existing MutableData (which is a lot more than 1 low level call).
1 Like
Heya, I believe we absolutely need a wrapper as suggested. Are you guys working on one somewhere? Is this it?
Wouldn’t it make sense to abstract common patterns into recipes?
- Basic initialization in one function:
init- takes care of basic bootstrapping: initialises, authorises and connects
- Bootstrapping user, with a general
authfunction or more granular control withsetupContainerandsetupService:
auth- checks if user has an account corresponding to a public id and if not create one: checks for access, gets containers (_publicand_publicNames), encrypts public id, …
- Reading and Updating mutable and immutable data
And so on, and so on. I haven’t gotten that deep into it yet. But it’s really making my head hurt trying to figure out how everything fits together for simple things like bootstrapping and service and reading and writing to the network.
Also I think code needs to be as clean and as readable as possible. Shouldn’t we be taking advantage that the browser has async/await native and write the wrapper using it’s clean syntax? The way we’ve started organizing our API utils in our project is staring to look pretty clean, check out this genNewAccount.js file. Let me know how I can help out with the wrapper
4 Likes
catch() is sort of a conditional branch that, once dealt with, will return to the following next()
But in this following next() only the second function for handling rejections will be fired, right? So the code relying on mdHandle shouldn’t be executed.
So catch() is one of those subtle things that can be confusing and it might not have been the best example to use.
First, let me say that if you do the following, A will print. The reason is that “catch” handles any exception and then switches back to the then track.
Promise.then(function () {throw 'OH NO';)
.catch(function (err) {console.log(err);})
.then(function () {console.log('A');});
Another note: once you have an exception, each then() in order is either skipped or it’s second argument is used: then(success, failure) is the same as then(success).catch(failure). You can have 15 then() and at the very end a single catch(). If the first then throws an exception, 14 thens are skipped, and the catch activates. For the longest time I thought you needed to catch on every then. What I’ve realized is that it’s best to catch only if you can fix the problem or you’re done executing.
Regardless of all that, if you don’t control the order of then calls, you cannot guarantee some then doesn’t get in between what returns mdHandle and what needs mdHandle. I think most people recognize this when they write their Promise chains relying on closure scoping to maintain their handle variable. The problem I have with that approach is that I find it difficult to avoid forking the Promise chain when nesting. If you’re already using class, and you’ve got some instance variables, why not use them? As soon as your App or MutableData or MutableDataEntry has a handle, the handle should get stored to that object. No need for closure nesting, no need to rely on the previous thenable returning your handle. It’s just there, in your object, ready to go. You also get the benefit of knowing exactly what to free() in each class.
1 Like
I don’t think anyone was suggesting we don’t also need a higher level abstraction. In particular, that is my intent in contributing to the linked codebase (see my pull request). I made a couple more modifications since the pull request so that I have code that looks like this:
let data = {'this': 'is', 'a': 'test'};
let app_info = JSON.parse(this.responseText);
let app = safeAPI.initialiseApp(app_info);
app.authoriseAndConnect({_public: ['Read']});
let md = app.quickRandomPublic(data);
md.getString('this').then(console.log);
md.getString('a').then(console.log);
let ent = md.getEntries();
ent.forEach(populate_file_list);
app.then(_ => console.log('All done!'));
md.free();
app.catch((err) => console.log);
app.free();
I think it’s super easy to read. It’s higher level. The Promise chain never forks, it’s a single chain with Promises resolved precisely in the order of calls as you read them. Other than authoriseAndConnect and quickRandomPublic, most of these calls are actually quite low level.
However, @loziniak is correct to point out it’s a bit muddled between low level calls and high level calls. It might be worth having a low level wrapper that tracks handles and prevents Promise forking, and then a higher abstraction. The problem with a higher abstraction is that, for example, quickRandomPublic as seen above would have sister functions quickRandomPrivate, quickPublic, and quickPrivate. Sticking with low level will limit the necessary functions to be defined by the number of functions made available by the API.
EDIT: I should have done ent.free() in that code.
DBL EDIT: getString is also higher level. It’s get() which then extracts buf.toString() from the return value.
1 Like
I strongly recommend looking into await/async syntax. There is a reason the SAFE examples have been rewritten by MaidSafe to step away from Promise.then/catch chaining.
2 Likes
I have a couple wish list items / things to consider for the wrapper.
When initializing, an app will try to read it’s last state or settings from a mutable data entry. On the first run of the app, this entry doesn’t exist, so an exception will occur. What I’ve seen several times is code relying on that exception occurring as an indication that the app needs to create the entry by inserting the default value. I don’t think that is quite right…I think you would also probably get the exception if internet connectivity was lost for example.
Another thing to consider is concurrent mutations. In the simple app that I wrote, I am trying to save the state of my app to a MD entry whenever the user changes something. I think problems can pop up when too many changes are made too quickly. I’m thinking maybe some sort of queueing would be the way to solve this, with only the most recent state needing to actually be recorded to the network. I was going to look into @happybeing’s apps that have ‘offline first’ storage, as that also seems like a good way to tackle it.
Great work guys!
4 Likes
Anyone want to collaborate in coming up with an API for a async/await wrapper library for the Safe web? I’ve created a quick repo websafe in case @drehb @bzee or everyone else would be willing to contribute
Here’s a first issue so we can discuss the initial API, please join!
4 Likes
As a quick update, I have submit two pull requests over time. Here’s the second one:
If you don’t want to wait for the PR to be accepted, the newest version of my code is available at my fork GitHub - btbonval/maidsafe-test. The following code pretty much works, though I’m still trying to flesh out the NFS bits.
let app = safeAPI.initialiseApp(app_info);
app.authoriseAndConnect({_public: ['Read']});
let md = app.quickRandomPublic({'this': 'is', 'a': 'test'});
md.getString('this').then(console.log);
md.getString('a').then(console.log);
let ent = md.getEntries();
ent.forEach(populate_file_list);
ent.free();
app.then(_ => console.log('All done!'));
md.free();
let store_md = app.quickRandomPublic({});
let directory = store_md.asNFS();
let file = directory.create('this is the data!');
file.metadata((data) => console.log(data)); // skipped?
file.free();
file = directory.create('more data fresh off the presses');
file.metadata((data) => console.log(data)); // skipped?
file.free();
file = directory.create('this data is on the network');
file.metadata((data) => console.log(data)); // skipped?
file.free();
ent = store_md.getEntries();
ent.forEach(populate_file_list); // nothing.
ent.len((size) => console.log(size)); // skipped?
ent.free();
directory.free();
store_md.free()
app.catch((err) => console.log(err));
app.free();
1 Like
Using async/await everywhere is a clear sign that SAFE API functions should be synchronous in my opinion.
1 Like
The SAFE API connects with the network, so a lot of calls are properly asynchronous. However, there are a lot of calls that one might expect to be synchronous. The reason for that are the FFI binding that MaidSafe utilised to make these APIs.
This might change in the future. I hope so at least. The APIs seems too much like it’s generated or copied mostly from the underlying SAFE libraries. At the moment it’s a fine solution though, it works.
The problem is that every synchronous code can be made asynchronous, but reverse is not possible (async/await functions always return promises). So unfortunately, we are locked in async world with no way out. For me API should give programmer a choice, which I don’t have in case of our API.
1 Like
It is possible. You can wrap any async call in a function that waits for it to finish thus making it sync.
1 Like
You mean by polling? something like this?
function syncCall() {
var a;
asyncCall().then((result) => { a = result; });
while (typeof a === 'undefined') { }
return a;
}
No.
But correct polling would work I guess. And this turns your async into a sync.
You have the same issue with turning a sync into a async function too.
Async functions allow you to have both async and sync functionality without the problems that you get when trying to use sync functions as async. (extra threads, checking, etc)