Allow plugins to surface output information

[ehmicky]

Follow-up from #711, #735, https://github.com/netlify/netlify-react-ui/issues/4285 and discussions with @mheffner, @vbrown608 and @Benaiah.

At the moment, plugins can only communicate the following information:

• print in the build logs
• modify environment variables for the next plugins or build.command
• any global state change such as writing on the filesystem or making network requests
• general status: success (if no error happened), failPlugin, failBuild, cancelBuild
• lots of error information on failure: error message, stack trace, error type, plugin metadata, etc.

We need to additionally let plugins communicate "outputs": summaries of success information, less verbose than build logs, and to be surfaced in the UI deploy summary.

1. One question we were unclear about: whether those outputs should allow including artifacts/blobs for download. The text below assumes no artifacts for GA.

2. Another question was about local/CLI builds. I am assuming the following: while outputs will be shown in the UI deploy summary, in local/CLI builds, they will be printed on the console instead, at the end of the build.

3. This is my suggestion for how Build plugins would do this. I think it is important to convey to plugin authors that this is not the same as doing a console.log(). In particular, we want to avoid overwhelming users with too many outputs in the UI. One way to convey those semantics (as discussed with @mheffner @vbrown608 @Benaiah) would be to make Build plugins return outputs.

module.exports = {
  async onInit() {
    await doSomething()
    return { output: 'Amazing output to show to the user' }
  }
}

An alternative would be to provide an utils method that can only be triggered once per event handler. This option might be more flexible to future product requirements (attaching artifacts, etc.), i.e. IMHO this might be better.

module.exports = {
  async onInit({ utils: { outputs } }) {
    await doSomething()
    outputs.show('Amazing output to show to the user')
    // This throws: Output already sent
    outputs.show('Other output')
  }
}

When using several event handlers, those outputs would be aggregated for a given plugin. Outputs would not be available as inputs of other plugins: the goal here is only to show information to users, not cross-plugin communication. The aggregated outputs would be send to Bitballoon API at the end of the build.

Please let me know what you think!

[erquhart]

Thanks for this @ehmicky - I think returning outputs is closest, as we only want a single summary from each plugin. But outputs could still be returned for multiple stages, so we may want to find a different way, like adding an onSuccess() lifecycle. Not sold on that being an ideal approach, but it at least exemplifies the outcome we want - outputs only being returned once for the entire plugin.

Also note that we'll likely want structured outputs, similar to the shape of summaries: https://github.com/netlify/netlify-react-ui/issues/4285#issuecomment-620675460

[ehmicky]

Note: we already have an onSuccess event handler. Are you suggesting we only allow exposing outputs in that event handler?

I think returning outputs is closest, as we only want a single summary from each plugin.

Note that we could still use a function, but throw if that function is called twice?

[iKristy]

Here is some explorations of the UI I was thinking regarding this! 👉 https://github.com/netlify/netlify-react-ui/issues/4285#issuecomment-624162909

[ehmicky]

About structured information: from this comment, I see the following fields:

• type (e.g. "info"): This would be specified as part of the method used by the plugin author. For example failBuild would result in an "error", while returning an output information would be an "info".
• title vs description vs details: sounds good. This means plugin authors would need to specify an object with those properties instead of a single string.
• I am assuming details is optional and title is required. Would description be optional?

Details on API endpoint to communicate this.

About ensuring plugins surface information only once:

• using onSuccess could work. However they would not get states from previous events unless they assign it to a variable in the file top-level scope.
• as you point out @shawn, using a return statements would not solve the fact that this could be done in several event handlers
• using a method that throws when repeated would most likely result in many build failures, which is not good

I suggest the following, which is simple and flexible IMHO:

• provide a utils.status.show() method
• if the method is called twice, the second call silently overwrites the first call

This also follows the pattern I have mostly seen in existing plugins. Many plugins are doing a console.log() at the end to notify for status. I think it would be more natural for plugin authors to just switch this to a different but similar method.

Please let me know what you think.

[verythorough]

I think your reasoning makes sense on all points, @ehmicky.

Regarding which fields should be required, we could actually make title optional, because the design includes a default version: "[plugin name] ran successfully". I agree that details can be optional, so that leaves description as probably being required. (Otherwise, why have an output at all?)

Regarding title, I wonder if heading might give people a better sense of the purpose/location of the content? I don't want them to think that they should just have the "title" of the plugin there. (Then again, "heading" is likely to be misspelled as "header", so unless we have an alias or something to handle it, maybe it's not worth the potential friction.)

[vbrown608]

I've got a V1 of the API endpoint for this here: https://www.notion.so/netlify/Plugin-Status-API-Spec-4721670531c1470aa95c5831338563d7

It seems like what's required for this endpoint (and the names for those things), is still changing quickly, but I wanted to get something out that Netlify Build and frontend can start working with. I'll update it as we hammer down naming and the shape of inputs.

Many of the names I went with for version 1 are shamelessly stolen from https://developer.github.com/v3/checks/runs/. I like that title, summary, and text have a clear hierarchy. To me, it's ambiguous whether details or description is more high-level.

[verythorough]

@vbrown608 That naming structure sounds great! Makes sense to keep consistency across platforms.

[ehmicky]

@verythorough This makes sense! So the API would be:

module.exports = {
  onPostBuild({ utils: { status } }) {
    doStuff()

    status.show({ 
      title: '5 files processed', 
      summary: 'We're done!', 
      text: 'Details on what happened', 
    })
  }
}

@vbrown608 Let's use those words :) Let's just make sure we are using those words also in front-end @nasivuela and product. Different words make things confusing.

So that would be:

• title, optional
• summary, required
• text, optional

[verythorough]

And for a bit more detail on the "optional" handling:

• title: optional; defaults to "[Plugin name] ran successfully"
• summary: required
• text: optional; if absent, deploy summary line is not expandable

Looking at GitHub's docs for their outputs object, I see that they set the title as required. I'd be totally fine with that, too, if we want to extend the consistency.

[vbrown608]

Totally makes sense to me from the perspective of the Netlify Build status helper. For the API endpoint in Bitballoon, which is only accessible by Netlify Build itself, I think none of those fields should be required so that Netlify Build can say, "It ran successfully, no other status information to report."

Note to myself for Monday: we should create a ticket about supporting rich text for some of those fields, eg via Markdown. Should involve frontend.

[ehmicky]

Actually from the perspective of the status helper, all arguments optional would be even simpler for plugin authors. I was more thinking from the standpoint of UI, thinking that showing only the default title [Plugin name] ran successfully might not be good enough user experience? Or is it? I am not sure. What do you think? That might be more a question for UI, design and product.

From the API standpoint, we definitely need all fields optional since some Build plugins won't call that status helper at all.

[iKristy]

For me, I +1 Jessica's suggestion above, but I would add I'm open to having the all the fields optional, with the caveat that we provide default messages for the title and summary if they don't fill them out.

[ehmicky]

I agree: providing a default summary would allow plugins not to call utils.status.show(), which would be simpler for them if they don't need to. Also, no plugins currently use that method (since it does not exist yet) and we need to support backward compatibility.

[nasivuela]

I see the naming has changed. FE can map the names to make it compatible with our current Deploy Summary Item. cc @charliegerard

From FE perspective:

• type => state (from a previously agreement with @vbrown608 )
• title => title
• description => summary
• summary (this is a concept on FE and HTML) => title + description
• details => text

Current optional/required on summary:

• title => required. If we make it optional we will need an extra call to plugins.json to get name of the plugin and build the default title. If plugin is not on the list, fallback to package name.

• description/summary is optional (markdown), but it looks like a defensive code strategy more than something expected. If I hide it, the alignment of title + icon is a little off. If we make it optional, an adjustment on the styles will be needed.

• details/text is optional (markdown).

[ehmicky]

I think we need to distinguish requiredness from a plugin author standpoint and from the front-end standpoint. For example, all fields could be optional for plugin authors (and they actually have to if we want to maintain backward compatibility with existing plugins). However, default values can be assigned somewhere in the middle (e.g. by the API) so that, from a front-end standpoint, they are always set without having to make an additional API call.

[ehmicky]

Current PRs to implement utils.status.show() at #1254, #1257, #1258, #1259. More PRs coming.

Also created issues:

• to add more state values at Allow users to differentiate between different types of plugin failures #1255
• to report more error types at Report more error types in the UI #1256

[ehmicky]

We discussed to confirm the following:

• the status properties are named title, summary and text
• when Build plugin authors are using utils.status.show(), we should enforce that summary is provided
• Build plugins do not have to use utils.status.show(), in which case a default status with { state: "success" } and no title, summary nor text will be sent to the API. This means those three fields are optional at the API level. This also means every Build plugin will report its status to the API, even if utils.status.show() is not used.
• The front-end should filter out the plugin statuses where summary is undefined and not display those

[vbrown608]

I would like to add one more point from my understanding, let me know if it's right:

• If summary is defined and title is blank, the frontend should construct a title like "Gatsby Cache Plugin succeeded". They should do so by using package to look up the plugin name.

[nasivuela]

For the UI

{ state: "success" }

• We'll add a row to output plugin info only if summary is defined.
• if title is undefined we'll show a default, example Plugin {name||package_name} run successfully.

{ state: "failed_build"||"canceled_build"||"failed_plugin" }

• We'll add a row always.
• If title is undefined we'll show a default, examples: {name||package_name} plugin failed due to an error but the build was able to continue / we couldn't deploy your site because of an error in {name||package_name} plugin
• Should we expect always a summary here?

[ehmicky]

For failed plugins: Yes, there should always be a summary there. Actually, there will always be a title too. The text is optional, but is very likely to be there as well.

I think we named the fourth state canceled_build?

[ehmicky]

This feature is now complete. There were quite many PRs (see the list above this comment) mostly related to:

• Adding utils.status.show() and validating its argument
• Sending this information to the API
• Ensuring plugins that do not use utils.status.show() still report a default status to the API
• Ensuring plugins calling utils.status.show() several times or failing after having called utils.status.show() report the correct status
• Creating the most helpful error messages that we can when reporting build errors on the deploy summary. There are many error types and we have lots of error information, but it had to tailored to the deploy summary.
• The later point also made it necessary to refactor error handling a little, to try to re-use code
• Making it work in CLI builds. Statuses are printed in build logs in CLI builds.
• Adding tests