Skip to content

Commit

Permalink
Merge pull request #226 from gentlementlegen/fix/logs
Browse files Browse the repository at this point in the history
fix: improved logs
  • Loading branch information
gentlementlegen authored Jan 9, 2025
2 parents bb72c0a + 25f8177 commit 32996e5
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 12 deletions.
3 changes: 2 additions & 1 deletion .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
"GHIJKLMNOPQRSTUVWXYZ",
"ubiquityos",
"azurefunc",
"marplex"
"marplex",
"ratelimit"
],
"dictionaries": ["typescript", "node", "software-terms"],
"import": ["@cspell/dict-typescript/cspell-ext.json", "@cspell/dict-node/cspell-ext.json", "@cspell/dict-software-terms"],
Expand Down
10 changes: 7 additions & 3 deletions src/github/handlers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function tryCatchWrapper(fn: (event: EmitterWebhookEvent) => unknown) {
try {
await fn(event);
} catch (error) {
console.error("Error in event handler", error);
console.error(`Error in event handler`, error, JSON.stringify(event));
}
};
}
Expand All @@ -29,7 +29,7 @@ export function bindHandlers(eventHandler: GitHubEventHandler) {

export async function shouldSkipPlugin(context: GitHubContext, pluginChain: PluginConfiguration["plugins"][0]) {
if (pluginChain.uses[0].skipBotEvents && "sender" in context.payload && context.payload.sender?.type === "Bot") {
console.log("Skipping plugin chain because sender is a bot");
console.log(`Skipping plugin ${JSON.stringify(pluginChain.uses[0].plugin)} in the chain because the sender is a bot`);
return true;
}
const manifest = await getManifest(context, pluginChain.uses[0].plugin);
Expand Down Expand Up @@ -65,10 +65,12 @@ async function handleEvent(event: EmitterWebhookEvent, eventHandler: InstanceTyp
const pluginChains = getPluginsForEvent(config.plugins, context.key);

if (pluginChains.length === 0) {
console.log(`No handler found for event ${event.name}`);
console.log(`No handler found for event ${event.name} (${context.key})`);
return;
}

console.log(`Will call the following chain:\n${pluginChains.map((o) => JSON.stringify(o.uses[0]?.plugin)).join("\n")}`);

for (const pluginChain of pluginChains) {
if (await shouldSkipPlugin(context, pluginChain)) {
continue;
Expand Down Expand Up @@ -100,6 +102,7 @@ async function handleEvent(event: EmitterWebhookEvent, eventHandler: InstanceTyp

// We wrap the dispatch so a failing plugin doesn't break the whole execution
try {
console.log(`Dispatching event for ${JSON.stringify(plugin)}`);
if (!isGithubPluginObject) {
await dispatchWorker(plugin, await inputs.getWorkerInputs());
} else {
Expand All @@ -111,6 +114,7 @@ async function handleEvent(event: EmitterWebhookEvent, eventHandler: InstanceTyp
inputs: await inputs.getWorkflowInputs(),
});
}
console.log(`Event dispatched for ${JSON.stringify(plugin)}`);
} catch (e) {
console.error(`An error occurred while processing the plugin chain, will skip plugin ${JSON.stringify(plugin)}`, e);
}
Expand Down
2 changes: 1 addition & 1 deletion src/github/handlers/repository-dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export async function repositoryDispatch(context: GitHubContext<"repository_disp
try {
pluginOutput = Value.Decode(pluginOutputSchema, context.payload.client_payload);
} catch (error) {
console.error("Cannot decode plugin output", error);
console.error(`Cannot decode plugin output from ${context.payload.repository.full_name}`, error);
throw error;
}
console.log("Plugin output", pluginOutput);
Expand Down
38 changes: 33 additions & 5 deletions src/github/utils/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@ export async function getConfigurationFromRepo(context: GitHubContext, repositor
repository,
owner,
});

if (!rawData) {
return { config: null, errors: null, rawData: null };
}

const { yaml, errors } = parseYaml(rawData);
const targetRepoConfiguration: PluginConfiguration | null = yaml;
console.log(`Will attempt to decode configuration for ${owner}/${repository}`);
if (targetRepoConfiguration) {
try {
const configSchemaWithDefaults = Value.Default(configSchema, targetRepoConfiguration) as Readonly<unknown>;
Expand All @@ -26,12 +32,14 @@ export async function getConfigurationFromRepo(context: GitHubContext, repositor
console.error(error);
}
}
return { config: Value.Decode(configSchema, configSchemaWithDefaults), errors, rawData };
const decodedConfig = Value.Decode(configSchema, configSchemaWithDefaults);
return { config: decodedConfig, errors, rawData };
} catch (error) {
console.error(`Error decoding configuration for ${owner}/${repository}, will ignore.`, error);
return { config: null, errors: [error instanceof TransformDecodeCheckError ? error.error : error] as ValueError[], rawData };
}
}
console.error(`YAML could not be decoded for ${owner}/${repository}`);
return { config: null, errors, rawData };
}

Expand Down Expand Up @@ -60,19 +68,28 @@ export async function getConfig(context: GitHubContext): Promise<PluginConfigura

let mergedConfiguration: PluginConfiguration = defaultConfiguration;

console.log(
`Will fetch configuration from ${payload.repository.owner.login}/${CONFIG_ORG_REPO}, ${payload.repository.owner.login}/${payload.repository.name}`
);
const configurations = await Promise.all([
getConfigurationFromRepo(context, CONFIG_ORG_REPO, payload.repository.owner.login),
getConfigurationFromRepo(context, payload.repository.name, payload.repository.owner.login),
]);

console.log(`Done fetching configurations for ${payload.repository.owner.login}/${payload.repository.name}, will merge them.`);

configurations.forEach((configuration) => {
if (configuration.config) {
mergedConfiguration = mergeConfigurations(mergedConfiguration, configuration.config);
}
});

console.log(`Will check plugin chains for ${payload.repository.owner.login}/${payload.repository.name}.`);

checkPluginChains(mergedConfiguration);

console.log(`Found ${mergedConfiguration.plugins.length} plugins enabled for ${payload.repository.owner.login}/${payload.repository.name}`);

for (const plugin of mergedConfiguration.plugins) {
const manifest = await getManifest(context, plugin.uses[0].plugin);
if (manifest) {
Expand Down Expand Up @@ -138,17 +155,28 @@ function checkExpression(value: string, allIds: Set<string>, calledIds: Set<stri
}

async function download({ context, repository, owner }: { context: GitHubContext; repository: string; owner: string }): Promise<string | null> {
if (!repository || !owner) throw new Error("Repo or owner is not defined");
if (!repository || !owner) {
console.error("Repo or owner is not defined, cannot download the requested file.");
return null;
}
const filePath = context.eventHandler.environment === "production" ? CONFIG_FULL_PATH : DEV_CONFIG_FULL_PATH;
try {
const { data } = await context.octokit.rest.repos.getContent({
console.log(`Attempting to fetch configuration for ${owner}/${repository}/${filePath}`);
const { data, headers } = await context.octokit.rest.repos.getContent({
owner,
repo: repository,
path: context.eventHandler.environment === "production" ? CONFIG_FULL_PATH : DEV_CONFIG_FULL_PATH,
path: filePath,
mediaType: { format: "raw" },
});
console.log(`Configuration file found at ${owner}/${repository}/${filePath}. xRateLimit remaining: ${headers?.["x-ratelimit-remaining"]}`);
return data as unknown as string; // this will be a string if media format is raw
} catch (err) {
console.error(err);
// In case of a missing config, do not log it as an error
if (err && typeof err === "object" && "status" in err && err.status === 404) {
console.log(`No configuration file was found at ${owner}/${repository}/${filePath}`);
} else {
console.error("Failed to download the requested file.", err);
}
return null;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/github/utils/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ async function fetchActionManifest(context: GitHubContext<"issue_comment.created
return manifest;
}
} catch (e) {
console.warn(`Could not find a manifest for Action ${owner}/${repo}: ${e}`);
console.error(`Could not find a manifest for Action ${owner}/${repo}: ${e}`);
}
return null;
}
Expand All @@ -54,7 +54,7 @@ async function fetchWorkerManifest(url: string): Promise<Manifest | null> {
_manifestCache[url] = manifest;
return manifest;
} catch (e) {
console.warn(`Could not find a manifest for Worker ${manifestUrl}: ${e}`);
console.error(`Could not find a manifest for Worker ${manifestUrl}: ${e}`);
}
return null;
}
Expand Down

0 comments on commit 32996e5

Please sign in to comment.