Comprehensive Error Handling Rehaul: Model + Router

[ktrott]

From a review meeting with Website team:

• Ergonomics around error handling and switching into boxValues mode not ideal as impacts your code significantly.
• Should the value of sentinels with $type error always be consistent as an Error object? Or should we allow client to control the value and leave it open-ended?
• If people want treatErrorAsValues, should we just suggest that they always model errors as an object so they can introspect to see that it's an error? Likely it would be better to provide a capital Error as it makes it just feel like JavaScript rather than special cased.

@DonutEspresso suggests that keeping client and server consistent in dealing with capital Error is best. Definitely feels more natural.

Needs more discussion.

[ktrott]

Establish patterns for dealing with errors: especially from get vs. call

[ktrott]

@jhusain rough notes from meeting with site team:

• Early warning of errors: Logging to consolidated logging
• Use error selector: When we log errors, log request ids along with them
• New feature: Log errors on a path level on the server-side for full stack trace debugging
• Backport Darren’s changes to the internal groovy script
• How do we tool around the server-side logging: meet with the Edge folks on it

[0xR]

I'm not sure if this is the right issue. But +1 on making error handling better. I don't get a stracktrace anywhere. Not on the server and not on the client. Also I don't think there is a way to get it.

I want to have access to the stacktrace only on the serverside. Exposing stacktraces to clients is bad practice.

Feature request: I want to be able to register an error handling function on the Router. This should be called with an Error instance with the path, error message and the stacktrace.

[ThePrimeagen]

@0xR We are definitely wanting to do this. We are going to re-think our error story holistically probably at the beginning of the new year. So please stay tuned. We will make it much better.

[przeor]

hello @michaelbpaulson @ktrott @sdesai

@Michael, I was in touch with your friend Gene Scheck with this topic, he told me that you are a very helpful guy.

I am writing a book for www.packtpub.com with full tech-stack as React+Node+Falcor+MongoDB.

This is will be 400 - 500 pages long book, and I need some feedback for you how to handle the errors in Falcor properly. From PacktPub we are getting content and technical's editorial help as well.

Regarding to this issue, my question is: how to handle auth errors for example?

How I can properly make an error in falcor-error and inform a user about it on the front-end in case when any of .set, .get, .call functions has an error (like not enough privileges for a certain route)?

@michaelbpaulson looking for your help o that topic.

P.S. as you see the screenshot from the falcor-router doc, it simply throw an error - but how to make it the was so a user can know what kind of error has happened? What is the best way in order to do it?

Regards, Kamil

[eddieajau]

An error use-case I'm running into is when you are using call to add multiple new records. For example, if we take the todos code example in the docs, I'm wanting to do something similar to:

// Inside Model
dataSource.
    call(
        // callPath parsed into Path array by Model
        ["todos","add"],
        // function arguments
        ["pick up some eggs", "a quart of milk", "and a stick of butter"], // <-- MULTI-ADD
        // refPaths parsed into PathSet arrays by Model
        [
            ["name"],
            ["done"]
        ])      

Say one of the todos fails a validation test. There does not appear to be any way to represent that one error in the returned JSON graph amongst the successful adds (which means at least for now I'm probably limited to adding only one todo at a time).

[drFabio]

I think a good approach would be if the result object of the call gave the raw response, and on the response you could send an error payload as a atom with $expires: 0 as a way to sinalize to not store it into cache since is just the operation metadata.

I tried but I couldn't get the atom on the response result, probably because the response comes from the already computed cache, longer expires would work but would polutte the cache.

I think that on the part of the calls it should exist an way to send non cache related data on the response payload.

[trxcllnt]

@drFabio the $expires: 0 atom not showing up in the response result is a known bug (falcor merges the atom into the cache, but the followup get sees it as expired, and throws it away without putting it into the response result). This bug has been squashed in our @graphistry forks of the falcor projects.

[drFabio]

@trxcllnt cool! Thanks. Have you submitted a PR for that or this branch is intended to diverge ?

[trxcllnt]

@drFabio that's up to the Netflix team ¯\(ツ)/¯ but I seriously doubt it. We've refactored or rewritten most of the client and router for better performance, deep React integration, and streaming data.

[steveorsomethin]

Update: this is paused, but in progress.