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

There should be a way to set an error callback per-require() #112

Closed
sylvinus opened this issue Jun 27, 2011 · 12 comments
Closed

There should be a way to set an error callback per-require() #112

sylvinus opened this issue Jun 27, 2011 · 12 comments

Comments

@sylvinus
Copy link

Maybe I misunderstood the doc, but there seems to be just one global require.onError() callback ?

I think there should be a simple way to do it with a require() call because I'm going to have to write glue code to redirect the global callback to the local ones based on the filenames...

thx!

@jrburke
Copy link
Member

jrburke commented Jun 28, 2011

You are correct, there is one global callback, require.onError(). There can be many require calls in play that need the same set of nested dependencies, and working out how to tie that back to a specific error callback would add more size to the loader. In addition, it does not make sense to have an error callback on a define()'d modules, and require calls are like define() calls, except they do not define a named script unit.

That said, if there is info you think is useful to have in the the onError() call, I can see about adding it. In master, if there is a runtime error calling the define() function callback, the error object will have as "moduleName" property that tells what module had the error.

@idris
Copy link

idris commented Aug 23, 2011

+1 for per-require error callbacks, or at least always including moduleName in the global onError(). If it adds size to the library, maybe it could be done as a plugin (though it's probably too core).

@unscriptable
Copy link

+1 on this. I've already fudged a solution for this. It's impossible to write a modular, robust web app without this. I'd prefer to use a CommonJS Promises/A implementation, but I know it's not nice to force everybody to implement promises. :) This seems like a reasonable extension to the AMD require function:

require(moduleList, callback, errback);

Thoughts?

@FrozenCow
Copy link

I agree with @unscriptable a simple failure-callback as an argument after the success-callback is very common and would still be compatible with existing require-calls.

@jrburke
Copy link
Member

jrburke commented Aug 23, 2011

I don't see it, at least for module loading -- it is a basic bootstrap to an operational page. It does not really matter where the error comes from, the page is likely not going to be fully usable. Either:

  1. You are in dev, and you want errors in loading to be very visible, you do not want them caught so you can get to the root error easily in the console. So you do not want any error catching.

  2. You have deployed the code for users. Since there can be nested dependencies, the error recovery case is likely just informing the user an error occurred and possibly a way to report it with some contextual info (like module name/path).

For #2, a global error handler is enough. I also do not like trying to accurately bind an error handler for each require call -- if two require() calls happen on a page, and they both have a common dependency that fails, you have to trace up the dependency chain to find which require calls should get notified.

In summary, I cannot see where a specific per-require helps improve error recovery. As mentioned, the error could be in a deep dependency, and it seems unlikely a per-require error handler would be able to do something useful vs. the global handler.

RequireJS 0.26.0 throws errors without catching them as the default behavior, and there is a catchError: {define: true} config option for catching errors when calling define() factory functions, and then require.onError would be called for those cases. The moduleName and url should be part of the error in that situation, so some contextual information is available.

I'm going to close this ticket for now, but feel free to continue commenting, particularly if there are use cases that can only be solved by per-require error handlers. We can reopen if something useful is uncovered.

@jrburke jrburke closed this as completed Aug 23, 2011
@unscriptable
Copy link

Hey James,

Just because a require() fails, doesn't mean the entire app is kaput. Here's a real-life example:

We've got a rather large app that's divided into several components that are loaded into a tab-like container on-demand. The specifications for these components (module id, permissions, etc) are loaded at boot time, and a require([moduleId], callback) is used to load the component when/if the user decides to use it.

Since it could take a few seconds to fetch and initialize the component, we put up a proxy component that shows a spinner and a text message, informing the user that something is happening. If the fetch for the component goes awry, we need to recover gracefully. That means removing the proxy component and replacing it with an error message.

In the future, we're planning to allow third parties to plug-in their own components. When we do this, the chance of an error increases, of course.

What are our alternatives if we don't have a local errback?

  • Prevent the user from attempting to fetch more than one component at a time. Therefore, we know that any global onError is for the current inflight component.
  • Allow the user to fetch more than one component at a time. We'd have to register all of the inflight components at some global level and figure out how to resolve which one(s) failed.
  • Implement a local setTimeout to detect the failure. Unfortunately, setTimeouts are problematic because you can't be sure how long a server resource is going to take to load: database contention, network congestion, etc.

Any global solution -- except the setTimeout solution which is local, but unreliable -- is considerably more work than having a local errback. Any other solution also increases the complexity and maintainability of the situation.

There are also third-party tools, such as wire.js, an IOC container, that could be used to assist in the loading of apps and/or on-demand-loaded components. These also need to clean up resources when bad stuff happens.

In general, cleaning up resources after an error condition is much better done in the same closure. Telling us that we should use a global onError is exactly the same situation as telling us we have no need of try/catch/finally because we already have window.onerror.

I could go on, but I don't mean to hijack your repo's issues list. If you don't want to discuss this here, I'll just move it over to the amd-implement group.

Regards,

-- John

@DylanArnold
Copy link

Old thread, but for reference.

I just started investigating whether it is possible to add an error callback per require. I am loading handlebars templates with the text plugin in a template collection object, I return a jquery Deferred which is resolved when the template is loaded.

It's something like this (untested variation of my code):

var dfds = {}; 
this.load = function(url) {
    var dfd = $.Deferred();

    if (dfds[url])
        return dfds[url]; // Found cached promise / template

    dfds[url] = dfd.promise();
    require(["text!" + url], function(template) {
        // Loaded template, resolve the deferred
        dfd.resolve(Handlebars.compile(template));
    });

    // dfd.reject(); // Error case???

    return dfd.promise();
};      

To me it kind of makes sense from a consistency point of view to handle the Deferreds failure case from the function caller.

When I thought about this more, sure it also makes sense that this should always work, so why bother with the failure case, it's not like I'm loading dynamic resources that are prone to failure.

However there still can be a failure case during a normal page load and that is when the webserver fails to deliver the template for whatever reason, high load or possibly the site went down for maintenance.

For a non essential part of the page, like a widget or something (like my current case), it could gracefully not render and not break by handling this.

I'm sure there may be more to this issue than meets the eye however, and I don't fully understand the global error handler because I only just came up against this issue, but my first impression is that this makes sense.

Cheers.

@jrburke
Copy link
Member

jrburke commented Sep 19, 2012

@DylanArnold per-require errbacks ended up getting into requirejs 2.0, but via another ticket/set of tickets, more detail here:

http://requirejs.org/docs/api.html#errbacks

@DylanArnold
Copy link

@jrburke Oh great stuff! Thanks for the link :)

@AkashSaikia
Copy link

Much needed! Thanks.

@jrburke
Copy link
Member

jrburke commented Sep 6, 2013

This is now possible for callback-style require() calls:
http://requirejs.org/docs/api.html#errbacks

@jonakyd
Copy link

jonakyd commented Nov 24, 2014

Hi, I know its an old thread,
but I currently bump into a error problem with CommonJS style.
Is it possible to use local error callback in CommonJS wrap's require()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants