1237575 - display docs link in default 404 handler for services

Status: RESOLVED FIXED

(Taskcluster :: General, defect)

Description

[Pete Moore [:pmoore][:pete]]

Sometimes people will stumble on scripts that connect to taskcluster APIs. They might not know about taskcluster, and may instinctively visit the top level domain. Currently, if they visit one of them, they will get a HTTP 404 with a payload "Cannot GET /".

Since we have docs for each service, it would make sense, at least from the top level path, to redirect to those docs.

Explicitly, let's set up the following HTTP 302 redirects:

http(s)://auth.taskcluster.net/             -> http://docs.taskcluster.net/auth/api-docs
http(s)://aws-provisioner.taskcluster.net/  -> http://docs.taskcluster.net/aws-provisioner/api-docs
http(s)://hooks.taskcluster.net/            -> http://docs.taskcluster.net/services/hooks
http(s)://index.taskcluster.net/            -> http://docs.taskcluster.net/services/index
http(s)://purge-cache.taskcluster.net/      -> http://docs.taskcluster.net/services/purge-cache
http(s)://queue.taskcluster.net/            -> http://docs.taskcluster.net/queue/api-docs
http(s)://scheduler.taskcluster.net/        -> http://docs.taskcluster.net/scheduler/api-docs
http(s)://secrets.taskcluster.net/          -> http://docs.taskcluster.net/services/secrets
http(s)://taskcluster-github.herokuapp.com/ -> http://docs.taskcluster.net/services/taskcluster-github

We could also consider redirecting from /v1 to the same docs location on all of the above domains.

This brings us a lot closer to the concept of "self describing apis", although of course not all the way.

Originally proposed here:
  * https://github.com/taskcluster/taskcluster-secrets/pull/6/files#r48695860

Comment 1

[Pete Moore [:pmoore][:pete]]

N.B. I've raised bug 1237585 to rename `taskcluster-github.herokuapp.com` to `github.taskcluster.net`.

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

I like this, but I'd like to rename the docs first (bug 1056282) which would change the redirect locations, so if you don't mind holding off that'd be great.

Comment 3

[Jonas Finnemann Jensen (:jonasfj)]

This should not be a 302 redirect!
It should be a 404, with HTML body that shows a link to the docs site.
If you want redirection from browser perspective use a meta-tag.

I want an API call that misses it's target to get a 404, that's a lot better than a redirect.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

While we're at it, the error pages should include Access-Control-Allow-Origin headers.  Right now if you get the API endpoints wrong in a browser, you just get a slew of same-origin failures, rather than a 404.

Comment 6

[Jonas Finnemann Jensen (:jonasfj)]

Updated title to be more accurate:
  Display link to docs in default 404 handler.

Comment 7

[Selena Deckelmann :selenamarie :selena]

Could we implement this now that bug 1056282 is closed?

Comment 8

[Yash Goyal]

Hi, I am new to open source . I would like to work on this bug as my first PR. Please guide me on how to get started.

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

Hi, Yash!  Welcome!

This is going to involve modifying a Javascript library, taskcluster-lib-app (https://github.com/taskcluster/taskcluster-lib-app), which all of our services use.  An example of a service is the hooks service, https://github.com/taskcluster/taskcluster-hooks.  It may also involve taskcluster-lib-docs.

Taskcluster-lib-app is usually initialized like this:

  server: {
    requires: ['cfg', 'api', 'docs'],
    setup: ({cfg, api, docs}) => {

      debug('Launching server.');
      let app = App(cfg.server);
      app.use('/v1', api);
      return app.createServer();
    },
  },

where cfg.server contains some options to the library.

The `docs` here is an instance of https://github.com/taskcluster/taskcluster-lib-docs 

I think the best way to implement this is to add two new options to App: `docs` and `rootDocsRedirect`.

The first, `docs`, should be the `docs` object.  The second, if true, should cause the root path `/` to render a 404 error message with some static HTML and a friendly message linking to the documentation.  This parameter should default to true, since we want it just about everywhere.

So: you will need to modify
 * taskcluster-lib-docs -- to provide a method that will return a link to the documentation
 * taskcluster-lib-app -- to handle these two new options

Once that's done, we'll upgrade the version of taskcluster-lib-app used in taskcluster-hooks and see the result at https://hooks.taskcluster.net!

Comment 10

[Yash Goyal]

I have been working on this bug and I am pretty sure that I will finish it up. Please assign this bug to me.

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

merged in https://github.com/taskcluster/taskcluster-lib-app/pull/19

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

Released as v3.0.0 (this is a breaking change, since docs must now be supplied).

Comment 13

[Pete Moore [:pmoore][:pete]]

Thanks Yash for implementing this!

Now we just need to upgrade our services to use v3, right?

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

Yes, and that's started in
  https://github.com/taskcluster/taskcluster-hooks/pull/51
  https://github.com/taskcluster/ec2-manager/pull/48

Comment 15

[Pete Moore [:pmoore][:pete]]

One of those PRs was closed, and the other has merge conflicts - what should we do here?

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

Yeah, this is done.  Not all services are upgraded -- I think ec2-manager's cleanup got abandoned -- but we're close enough.