Skip to content

Rutger/fix user error trace #42

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.

Sign up for GitHub

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

Merged
parutger merged 4 commits into from
Feb 17, 2023
Merged

Rutger/fix user error trace #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
761e3e2
Change the localtest address from localhost to 127.0.0.1
parutger Feb 15, 2023
b641e1c
Add dotenv/config to nodemon launch parameters
parutger Feb 15, 2023
542629b
Improves error handling and user message
parutger Feb 17, 2023
9f0c5bd
Applies formatting rules to src/tip.ts
parutger Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion nodemon.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
"watch": ["src/"],
"ignore": ["test/", "node_modules/", ".git"],
"ext": ".ts",
"exec": "node -r ts-node/register src/bot.ts"
"exec": "node -r ts-node/register -r dotenv/config src/bot.ts"
}
17 changes: 14 additions & 3 deletions src/bot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { displayError, envVar } from "opstooling-js";
import { Probot, run } from "probot";

import { getTipSize, parseContributorAccount, tipUser } from "./tip";
import { State } from "./types";
import { ContributorAccount, State } from "./types";

const onIssueComment = async (
state: State,
Expand Down Expand Up @@ -41,8 +41,19 @@ const onIssueComment = async (
return `You are not allowed to request a tip. Only members of ${allowedGitHubOrg}/${allowedGitHubTeam} are allowed.`;
}

const contributorAccount = parseContributorAccount(pullRequestBody);
const tipSize = getTipSize(tipSizeInput);
let contributorAccount: ContributorAccount;
try {
contributorAccount = parseContributorAccount(pullRequestBody);
} catch (error) {
return error.message;
}

let tipSize: string;
try {
tipSize = getTipSize(tipSizeInput);
} catch (error) {
return error.message;
Copy link
Contributor

@mordamax mordamax Feb 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That covers only these 2 methods, but it doesn't for example tipUser or anything else.
I would propose to look into how respondOnResult writes the message in ALL cases, and make sure the trace isn't printed there

}

bot.log(
`Valid command!\n${tipRequester} wants to tip ${contributorLogin} (${contributorAccount.address} on ${contributorAccount.network}) a ${tipSize} tip for pull request ${pullRequestUrl}.`,
Expand Down
6 changes: 3 additions & 3 deletions src/tip.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export async function tipUser(

const reason = `TO: ${contributor.githubUsername} FOR: ${pullRequestRepo}#${pullRequestNumber} (${tipSize})`;
/* TODO before submitting, check tip does not already exist via a storage query.
TODO potentially prevent duplicates by also checking for reasons with the other sizes. */
TODO potentially prevent duplicates by also checking for reasons with the other sizes. */
const unsub = await api.tx.tips
.reportAwesome(reason, contributor.account.address)
.signAndSend(botTipAccount, (result: SubmittableResult) => {
Expand All @@ -95,7 +95,7 @@ export async function tipUser(
unsub();
}
});

await api.disconnect();
Copy link
Contributor

@mordamax mordamax Feb 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this solve the reconnection or runtime upgrade issue?

Copy link
Contributor Author

@parutger parutger Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, But I am hoping this will solve the memory leak issue that was reported.

return { success: true, tipUrl };
}

Expand All @@ -108,7 +108,7 @@ async function getContributorMetadata(state: State, contributor: Contributor): P
switch (contributor.account.network) {
case "localtest": {
return {
provider: new WsProvider("ws://localhost:9944"),
provider: new WsProvider("ws://127.0.0.1:9944"),
botTipAccount: keyring.addFromUri("//Alice", { name: "Alice default" }),
tipUrl: "https://polkadot.js.org/apps/?rpc=ws%3A%2F%2F127.0.0.1%3A9944#/treasury/tips",
};
Expand Down